[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
Wed Jul 29 09:33:05 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);
----------------
jdenny wrote:
> 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.
9f2f3b9de6314a009322b6081c792ebf9a469460 relands this patch with the latter solution.


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