[PATCH] D83635: [OpenMPOpt][WIP] Merge parallel regions

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:514
     Changed |= deleteParallelRegions();
+    Changed |= deduplicateRuntimeCalls();
+    Changed |= mergeParallelRegions();
----------------
ggeorgakoudis wrote:
> I have intentionally added re-running deduplication before merging. It helps removing emitted calls to `__kmpc_global_thread_num`, and that enables merging of parallel regions by removing redundant in-between instructions (at least at this point that merging is very defensive)
looks good.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:631
+                        << " parallel regions in " << OriginalFn->getName()
+                        << "\n");
+
----------------
ggeorgakoudis wrote:
> jdoerfert wrote:
> > We should also emit a remark here. And one pointing at each of the merged parallel regions.
> We emit a remark in line 708 that identifies merged parallel regions. What do you have in mind? Is it a single remark that contains identifiers for all merged regions?
I guess one per parallel region with the first one saying "merged the following parallel region into this one", and the others saying "merged into the parallel region above". The one we have below just says "we merged something" (IIRC).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83635