[PATCH] D82722: [OpenMP][IRBuilder] Support nested parallel regions

2020-07-14 Thread Johannes Doerfert via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7af287d0d921: [OpenMP][IRBuilder] Support nested parallel 
regions (authored by jdoerfert).

Changed prior to commit:
  https://reviews.llvm.org/D82722?vs=273959&id=278061#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82722

Files:
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/irbuilder_nested_openmp_parallel_empty.c
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -127,13 +127,16 @@
 void OpenMPIRBuilder::initialize() { initializeTypes(M); }
 
 void OpenMPIRBuilder::finalize() {
+  SmallPtrSet ParallelRegionBlockSet;
+  SmallVector Blocks;
   for (OutlineInfo &OI : OutlineInfos) {
-assert(!OI.Blocks.empty() &&
-   "Outlined regions should have at least a single block!");
-BasicBlock *RegEntryBB = OI.Blocks.front();
-Function *OuterFn = RegEntryBB->getParent();
+ParallelRegionBlockSet.clear();
+Blocks.clear();
+OI.collectBlocks(ParallelRegionBlockSet, Blocks);
+
+Function *OuterFn = OI.EntryBB->getParent();
 CodeExtractorAnalysisCache CEAC(*OuterFn);
-CodeExtractor Extractor(OI.Blocks, /* DominatorTree */ nullptr,
+CodeExtractor Extractor(Blocks, /* DominatorTree */ nullptr,
 /* AggregateArgs */ false,
 /* BlockFrequencyInfo */ nullptr,
 /* BranchProbabilityInfo */ nullptr,
@@ -143,6 +146,8 @@
 /* Suffix */ ".omp_par");
 
 LLVM_DEBUG(dbgs() << "Before outlining: " << *OuterFn << "\n");
+LLVM_DEBUG(dbgs() << "Entry " << OI.EntryBB->getName()
+  << " Exit: " << OI.ExitBB->getName() << "\n");
 assert(Extractor.isEligible() &&
"Expected OpenMP outlining to be possible!");
 
@@ -162,12 +167,12 @@
 // made our own entry block after all.
 {
   BasicBlock &ArtificialEntry = OutlinedFn->getEntryBlock();
-  assert(ArtificialEntry.getUniqueSuccessor() == RegEntryBB);
-  assert(RegEntryBB->getUniquePredecessor() == &ArtificialEntry);
-  RegEntryBB->moveBefore(&ArtificialEntry);
+  assert(ArtificialEntry.getUniqueSuccessor() == OI.EntryBB);
+  assert(OI.EntryBB->getUniquePredecessor() == &ArtificialEntry);
+  OI.EntryBB->moveBefore(&ArtificialEntry);
   ArtificialEntry.eraseFromParent();
 }
-assert(&OutlinedFn->getEntryBlock() == RegEntryBB);
+assert(&OutlinedFn->getEntryBlock() == OI.EntryBB);
 assert(OutlinedFn && OutlinedFn->getNumUses() == 1);
 
 // Run a user callback, e.g. to add attributes.
@@ -614,20 +619,12 @@
   InsertPointTy PreFiniIP(PRegPreFiniBB, PRegPreFiniTI->getIterator());
   FiniCB(PreFiniIP);
 
-  SmallPtrSet ParallelRegionBlockSet;
-  SmallVector Worklist;
-  ParallelRegionBlockSet.insert(PRegEntryBB);
-  ParallelRegionBlockSet.insert(PRegExitBB);
+  OI.EntryBB = PRegEntryBB;
+  OI.ExitBB = PRegExitBB;
 
-  // Collect all blocks in-between PRegEntryBB and PRegExitBB.
-  Worklist.push_back(PRegEntryBB);
-  while (!Worklist.empty()) {
-BasicBlock *BB = Worklist.pop_back_val();
-OI.Blocks.push_back(BB);
-for (BasicBlock *SuccBB : successors(BB))
-  if (ParallelRegionBlockSet.insert(SuccBB).second)
-Worklist.push_back(SuccBB);
-  }
+  SmallPtrSet ParallelRegionBlockSet;
+  SmallVector Blocks;
+  OI.collectBlocks(ParallelRegionBlockSet, Blocks);
 
   // Ensure a single exit node for the outlined region by creating one.
   // We might have multiple incoming edges to the exit now due to finalizations,
@@ -635,10 +632,10 @@
   BasicBlock *PRegOutlinedExitBB = PRegExitBB;
   PRegExitBB = SplitBlock(PRegExitBB, &*PRegExitBB->getFirstInsertionPt());
   PRegOutlinedExitBB->setName("omp.par.outlined.exit");
-  OI.Blocks.push_back(PRegOutlinedExitBB);
+  Blocks.push_back(PRegOutlinedExitBB);
 
   CodeExtractorAnalysisCache CEAC(*OuterFn);
-  CodeExtractor Extractor(OI.Blocks, /* DominatorTree */ nullptr,
+  CodeExtractor Extractor(Blocks, /* DominatorTree */ nullptr,
   /* AggregateArgs */ false,
   /* BlockFrequencyInfo */ nullptr,
   /* BranchProbabilityInfo */ nullptr,
@@ -694,7 +691,7 @@
 
   LLVM_DEBUG(dbgs() << "After  privatization: " << *OuterFn << "\n");
   LLVM_DEBUG({
-for (auto *BB : OI.Blocks)
+for (auto *BB : Blocks)
   dbgs() << " PBR: " << BB->getName() << "\n";
   });
 
@@ -1112,3 +1109,20 @@
   VarName##Ptr = PointerType::getUnqual(T);
 #include "llvm/Frontend/OpenMP/OMPKinds.def"
 }
+
+void OpenMPIRBuilder::OutlineInfo::collectBlocks(
+SmallPtrSetImpl &BlockSet,
+SmallVe

[PATCH] D82722: [OpenMP][IRBuilder] Support nested parallel regions

2020-06-29 Thread Fady Ghanim via Phabricator via cfe-commits 
fghanim added a comment.

OK. Thanks :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82722



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82722: [OpenMP][IRBuilder] Support nested parallel regions

2020-06-29 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert added a comment.

In D82722#2118958 , @fghanim wrote:

> Thanks for working on this. LGTM.
>  Did you make any changes other than splitting from D82470 
>  ?


No.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82722



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82722: [OpenMP][IRBuilder] Support nested parallel regions

2020-06-29 Thread Fady Ghanim via Phabricator via cfe-commits 
fghanim added a comment.

Thanks for working on this. LGTM.
Did you make any changes other than splitting from D82470 
 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82722



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82722: [OpenMP][IRBuilder] Support nested parallel regions

2020-06-29 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert created this revision.
jdoerfert added reviewers: anchu-rajendran, kiranktp, fghanim.
Herald added a subscriber: rogfer01.
Herald added projects: clang, LLVM.
jdoerfert added a child revision: D82470: [OpenMP][IRBuilder] Support allocas 
in nested parallel regions.

During code generation we might change/add basic blocks so keeping a
list of them is fairly easy to break. Nested parallel regions were
enough. The new scheme does recompute the list of blocks to be outlined
once it is needed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82722

Files:
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/irbuilder_nested_openmp_parallel_empty.c
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -128,13 +128,16 @@
 void OpenMPIRBuilder::initialize() { initializeTypes(M); }
 
 void OpenMPIRBuilder::finalize() {
+  SmallPtrSet ParallelRegionBlockSet;
+  SmallVector Blocks;
   for (OutlineInfo &OI : OutlineInfos) {
-assert(!OI.Blocks.empty() &&
-   "Outlined regions should have at least a single block!");
-BasicBlock *RegEntryBB = OI.Blocks.front();
-Function *OuterFn = RegEntryBB->getParent();
+ParallelRegionBlockSet.clear();
+Blocks.clear();
+OI.collectBlocks(ParallelRegionBlockSet, Blocks);
+
+Function *OuterFn = OI.EntryBB->getParent();
 CodeExtractorAnalysisCache CEAC(*OuterFn);
-CodeExtractor Extractor(OI.Blocks, /* DominatorTree */ nullptr,
+CodeExtractor Extractor(Blocks, /* DominatorTree */ nullptr,
 /* AggregateArgs */ false,
 /* BlockFrequencyInfo */ nullptr,
 /* BranchProbabilityInfo */ nullptr,
@@ -144,6 +147,8 @@
 /* Suffix */ ".omp_par");
 
 LLVM_DEBUG(dbgs() << "Before outlining: " << *OuterFn << "\n");
+LLVM_DEBUG(dbgs() << "Entry " << OI.EntryBB->getName()
+  << " Exit: " << OI.ExitBB->getName() << "\n");
 assert(Extractor.isEligible() &&
"Expected OpenMP outlining to be possible!");
 
@@ -163,12 +168,12 @@
 // made our own entry block after all.
 {
   BasicBlock &ArtificialEntry = OutlinedFn->getEntryBlock();
-  assert(ArtificialEntry.getUniqueSuccessor() == RegEntryBB);
-  assert(RegEntryBB->getUniquePredecessor() == &ArtificialEntry);
-  RegEntryBB->moveBefore(&ArtificialEntry);
+  assert(ArtificialEntry.getUniqueSuccessor() == OI.EntryBB);
+  assert(OI.EntryBB->getUniquePredecessor() == &ArtificialEntry);
+  OI.EntryBB->moveBefore(&ArtificialEntry);
   ArtificialEntry.eraseFromParent();
 }
-assert(&OutlinedFn->getEntryBlock() == RegEntryBB);
+assert(&OutlinedFn->getEntryBlock() == OI.EntryBB);
 assert(OutlinedFn && OutlinedFn->getNumUses() == 1);
 
 // Run a user callback, e.g. to add attributes.
@@ -618,20 +623,12 @@
   InsertPointTy PreFiniIP(PRegPreFiniBB, PRegPreFiniTI->getIterator());
   FiniCB(PreFiniIP);
 
-  SmallPtrSet ParallelRegionBlockSet;
-  SmallVector Worklist;
-  ParallelRegionBlockSet.insert(PRegEntryBB);
-  ParallelRegionBlockSet.insert(PRegExitBB);
+  OI.EntryBB = PRegEntryBB;
+  OI.ExitBB = PRegExitBB;
 
-  // Collect all blocks in-between PRegEntryBB and PRegExitBB.
-  Worklist.push_back(PRegEntryBB);
-  while (!Worklist.empty()) {
-BasicBlock *BB = Worklist.pop_back_val();
-OI.Blocks.push_back(BB);
-for (BasicBlock *SuccBB : successors(BB))
-  if (ParallelRegionBlockSet.insert(SuccBB).second)
-Worklist.push_back(SuccBB);
-  }
+  SmallPtrSet ParallelRegionBlockSet;
+  SmallVector Blocks;
+  OI.collectBlocks(ParallelRegionBlockSet, Blocks);
 
   // Ensure a single exit node for the outlined region by creating one.
   // We might have multiple incoming edges to the exit now due to finalizations,
@@ -639,10 +636,10 @@
   BasicBlock *PRegOutlinedExitBB = PRegExitBB;
   PRegExitBB = SplitBlock(PRegExitBB, &*PRegExitBB->getFirstInsertionPt());
   PRegOutlinedExitBB->setName("omp.par.outlined.exit");
-  OI.Blocks.push_back(PRegOutlinedExitBB);
+  Blocks.push_back(PRegOutlinedExitBB);
 
   CodeExtractorAnalysisCache CEAC(*OuterFn);
-  CodeExtractor Extractor(OI.Blocks, /* DominatorTree */ nullptr,
+  CodeExtractor Extractor(Blocks, /* DominatorTree */ nullptr,
   /* AggregateArgs */ false,
   /* BlockFrequencyInfo */ nullptr,
   /* BranchProbabilityInfo */ nullptr,
@@ -698,7 +695,7 @@
 
   LLVM_DEBUG(dbgs() << "After  privatization: " << *OuterFn << "\n");
   LLVM_DEBUG({
-for (auto *BB : OI.Blocks)
+for (auto *BB : Blocks)
   dbgs() << " PBR: " << BB->getName() << "\n";
   });
 
@@ -996,3 +993,20 @@
   std::string Name = g