[Openmp-commits] [PATCH] D101030: [OpenMP] Overhaul `declare target` handling

[Alexey Bataev via Phabricator via Openmp-commits]

ABataev added a comment.

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.



================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:2999-3000
     if (!hasTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum))
-      initializeTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum,
-                                      OffloadingEntriesNum);
     auto &Entry =
----------------
Why did you decide to drop this?


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:1736
   SkipUntil(tok::annot_pragma_openmp_end, StopBeforeMatch);
-  ConsumeAnyToken();
-  for (auto &MTLocDecl : DeclareTargetDecls) {
----------------
Looks like it is still required to consume `tok::annot_pragma_openmp_end`


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:1742
+        Sema::DeclareTargetContextInfo::MapInfo MI{MT, NameInfo.getLoc()};
+        bool FirstMapping = DTCI.ExplicitlyMapped.insert({ND, MI}).second;
+        if (!FirstMapping)
----------------
`try_emplace(ND, MI)`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101030/new/

https://reviews.llvm.org/D101030