[PATCH] D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)

Joel E. Denny via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 28 17:54:21 PDT 2020


jdenny added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8055
       for (const auto L : C->component_lists()) {
-        InfoGen(std::get<0>(L), std::get<1>(L), OMPC_MAP_to, llvm::None,
+        SmallVector<OpenMPMapModifierKind, 2> MapModifiers;
+        translateMotionModifiers(C->getMotionModifiers(), MapModifiers);
----------------
I haven't managed to locally reproduce the bot failures I saw today when trying to push this patch.  Somehow they're dropping the present modifiers during codegen.  I think it's due to memory corruption that my machines aren't managing to experience.  The culprit seems to be that `MapModifiers` is local here, but the `InfoGen` call below stores it in a `MapInfo` as an `ArrayRef`, which becames a dangling ref by the time it's used.

One solution is to change `MapInfo` to store a `SmallVector` instead.  Another would be for `MapInfo` to store a second `ArrayRef` for `C->getMotionModifiers()`, which would be translated to runtime map flags later.  I'm planning to try the latter solution tomorrow.  I prefer it because it seems more space efficient and because it translates motion modifiers directly to runtime flags instead of translating to map type modifiers first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84711



More information about the cfe-commits mailing list