[PATCH] D90909: [OpenMPOpt][WIP] Expand parallel region merging

[Giorgis Georgakoudis via Phabricator via llvm-commits]

ggeorgakoudis added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:670
+          // Find inputs/outputs to/from the sequential region. Convert inputs
+          // to pointer uses in the sequential region. Broadcast outputs to
+          // users outside the soon-to-be-merged region. Sinking/hoisting
----------------
jdoerfert wrote:
> Conversion to pointer happens in the IRBuilder (after D92189 landed). This means we can omit the input alloca stuff below, right?
That's correct. Removed.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:974
+            if (!I.isLifetimeStartOrEnd())
+              return false;
+          }
----------------
jdoerfert wrote:
> A lot of heuristics could be used until we have an AAOpenMPRuntimeCalls attribute that determines if a call can result in an OpenMP runtime call.
> Lifetime intrinisics is fine, any intrinsic should be OK actually.
Makes sense. Updated to accept any intrinsic call as mergable.


================
Comment at: llvm/test/Transforms/OpenMP/parallel_region_merging_legacy_pm.ll:412
-; CHECK-NEXT:    ret void
-;
----------------
jdoerfert wrote:
> Why was this file deleted?
There were different test files for the legacy and the new PMs because of them processing changes in the CG in opposite order, so function names for outlined functions wouldn't match. This has been resolved in D90566,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90909