jdoerfert marked an inline comment as done. jdoerfert added a comment. In D101030#2708277 <https://reviews.llvm.org/D101030#2708277>, @ABataev wrote:
> Could you check that it does not break the tests from > https://github.com/clang-ykt/omptests? IIRC, "ref" was introduced to fix a > bug with too early optimizations of the declare target variables defined in > other modules. Check `t-same-name-definitions` especially, though I'm not > sure that exactly this test caused adding of `$ref`s. I did run that test, works fine. The "$ref"s are still generated, except if you don't have a host version of a variable. Without it, there is no reason to have a ref because you cannot actually address the device version. ================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:2999-3000 if (!hasTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum)) - initializeTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum, - OffloadingEntriesNum); auto &Entry = ---------------- ABataev wrote: > Why did you decide to drop this? Because a missing host version should mean we do not create a region entry. Take a look at all the situations in `clang/test/OpenMP/declare_target_only_one_side_compilation.cpp` for which we are missing a host entry. None of them should create a region entry info. ================ Comment at: clang/lib/Parse/ParseOpenMP.cpp:1736 SkipUntil(tok::annot_pragma_openmp_end, StopBeforeMatch); - ConsumeAnyToken(); - for (auto &MTLocDecl : DeclareTargetDecls) { ---------------- ABataev wrote: > Looks like it is still required to consume `tok::annot_pragma_openmp_end` That's done in the caller now because the clauses are not always parsed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101030/new/ https://reviews.llvm.org/D101030 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
