[PATCH] D129285: [OMPIRBuilder] Fill the Aggregate Struct in the serialized parallel region

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1054
+        Builder.CreateStore(StoredAggValue, CloneGEP);
+      }
       CI->removeFromParent();
----------------
This is super fragile. What if there is no GEP for 0 offset or a GEP is reused. If we want something like this we should verify the conditions more closely. E.g., GEP->num_uses() == 1, all users of the CapturedStruct should be GEPs or other things we expect, etc.

Honestly, I would suggest to unify the handling rather than adding special cases. So, always outline, always do the same thing. In the serial case just omit the fork_call indirection.

Would that work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129285