[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-13 Thread Puyan Lotfi via Phabricator via cfe-commits 
plotfi added a comment.

In D74372#1873665 , @jdoerfert wrote:

> In D74372#1873623 , @plotfi wrote:
>
> > Could this be reverted for the time being so that bots can go green? There 
> > appear to be an awful lot of commits piling up on top of this one.
>
>
> Should be fixed by now but you should revert if you feel it causes a problem.


Nice, thank you! 
https://github.com/llvm/llvm-project/commit/3f3ec9c40b2574929b41b93ac38484081b49837b
 did the trick.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-12 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert added a comment.

In D74372#1873623 , @plotfi wrote:

> Could this be reverted for the time being so that bots can go green? There 
> appear to be an awful lot of commits piling up on top of this one.


Should be fixed by now but you should revert if you feel it causes a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-12 Thread Puyan Lotfi via Phabricator via cfe-commits 
plotfi added a comment.

Could this be reverted for the time being so that bots can go green? There 
appear to be an awful lot of commits piling up on top of this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-12 Thread Vlad Vereschaka via Phabricator via cfe-commits 
vvereschaka added a comment.

it also gets failed on the Linux builders

http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-ubuntu/builds/2843
http://lab.llvm.org:8011/builders/lld-x86_64-ubuntu-fast/builds/11581


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-12 Thread Vlad Vereschaka via Phabricator via cfe-commits 
vvereschaka added a comment.

Hello @jdoerfert,

`OpenMPIRBuilderTest.ParallelCancelBarrier` is failed on Windows builders:

http://lab.llvm.org:8011/builders/llvm-clang-x86_64-win-fast/builds/11242/steps/test-check-llvm-unit/logs/stdio
http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/4537
http://lab.llvm.org:8011/builders/llvm-clang-win-x-aarch64/builds/4534

  
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\unittests\Frontend\OpenMPIRBuilderTest.cpp(612):
 error:   Expected: CI->getNextNode()->getSuccessor(0)
  
Which is: 00DC6479BEC0
  
  To be equal to: ExitBB
  
Which is: 00DC6479B140
  
  [  FAILED  ] OpenMPIRBuilderTest.ParallelCancelBarrier (4 ms)

Would you you fix the test or revert this commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-12 Thread Johannes Doerfert via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a56d64d7620: [OpenMP][IRBuilder] Perform finalization 
(incl. outlining) late (authored by jdoerfert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -96,7 +96,7 @@
   EXPECT_EQ(cast(Barrier)->getArgOperand(1), GTID);
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancel) {
@@ -151,7 +151,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancelIfCond) {
@@ -212,7 +212,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancelBarrier) {
@@ -267,7 +267,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, DbgLoc) {
@@ -362,9 +362,9 @@
 
   auto FiniCB = [&](InsertPointTy CodeGenIP) { ++NumFinalizationPoints; };
 
-  IRBuilder<>::InsertPoint AfterIP = OMPBuilder.CreateParallel(
-  Loc, BodyGenCB, PrivCB, FiniCB, nullptr, nullptr, OMP_PROC_BIND_default, false);
-
+  IRBuilder<>::InsertPoint AfterIP =
+  OMPBuilder.CreateParallel(Loc, BodyGenCB, PrivCB, FiniCB, nullptr,
+nullptr, OMP_PROC_BIND_default, false);
   EXPECT_EQ(NumBodiesGenerated, 1U);
   EXPECT_EQ(NumPrivatizedVars, 1U);
   EXPECT_EQ(NumFinalizationPoints, 1U);
@@ -372,10 +372,12 @@
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
 
+  OMPBuilder.finalize();
+
   EXPECT_NE(PrivAI, nullptr);
   Function *OutlinedFn = PrivAI->getFunction();
   EXPECT_NE(F, OutlinedFn);
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
   EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoUnwind));
   EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoRecurse));
   EXPECT_TRUE(OutlinedFn->hasParamAttribute(0, Attribute::NoAlias));
@@ -470,6 +472,7 @@
 
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
+  OMPBuilder.finalize();
 
   EXPECT_NE(PrivAI, nullptr);
   Function *OutlinedFn = PrivAI->getFunction();
@@ -595,6 +598,7 @@
 
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
+  OMPBuilder.finalize();
 
   EXPECT_FALSE(verifyModule(*M, ()));
 
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -1405,11 +1405,7 @@
   DISubprogram *OldSP = OldFunc.getSubprogram();
   LLVMContext  = OldFunc.getContext();
 
-  // See llvm.org/PR44560, OpenMP passes an invalid subprogram to CodeExtractor.
-  bool NeedWorkaroundForOpenMPIRBuilderBug =
-  OldSP && OldSP->getRetainedNodes()->isTemporary();
-
-  if (!OldSP || NeedWorkaroundForOpenMPIRBuilderBug) {
+  if (!OldSP) {
 // Erase any debug info the new function contains.
 stripDebugInfo(NewFunc);
 // Make sure the old function doesn't contain any non-local metadata refs.
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -93,6 +93,55 @@
 
 void OpenMPIRBuilder::initialize() { initializeTypes(M); }
 
+void OpenMPIRBuilder::finalize() {
+  for (OutlineInfo  : OutlineInfos) {
+assert(!OI.Blocks.empty() &&
+   "Outlined regions should have at least a single block!");
+BasicBlock *RegEntryBB = OI.Blocks.front();
+Function *OuterFn = RegEntryBB->getParent();
+CodeExtractorAnalysisCache CEAC(*OuterFn);
+CodeExtractor Extractor(OI.Blocks, /* DominatorTree */ nullptr,
+/* AggregateArgs */ false,
+/* BlockFrequencyInfo */ nullptr,
+/* BranchProbabilityInfo */ nullptr,
+/* AssumptionCache */ nullptr,
+/* AllowVarArgs */ true,
+/* AllowAlloca */ true,
+/* Suffix */ ".omp_par");
+
+LLVM_DEBUG(dbgs() << "Before outlining: " << *OuterFn << "\n");
+
+

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-12 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield accepted this revision.
JonChesterfield added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert added a comment.

I think this fixes the bug and allows loop optimizations later, let me know if 
there is anything else


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Vedant Kumar via Phabricator via cfe-commits 
vsk added inline comments.



Comment at: llvm/lib/Transforms/Utils/CodeExtractor.cpp:1408
 
-  // See llvm.org/PR44560, OpenMP passes an invalid subprogram to 
CodeExtractor.
-  bool NeedWorkaroundForOpenMPIRBuilderBug =
-  OldSP && OldSP->getRetainedNodes()->isTemporary();
-
-  if (!OldSP || NeedWorkaroundForOpenMPIRBuilderBug) {
+  if (!OldSP) {
 // Erase any debug info the new function contains.

This part lgtm, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert updated this revision to Diff 244005.
jdoerfert added a comment.

Improve comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -96,7 +96,7 @@
   EXPECT_EQ(cast(Barrier)->getArgOperand(1), GTID);
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancel) {
@@ -151,7 +151,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancelIfCond) {
@@ -212,7 +212,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancelBarrier) {
@@ -267,7 +267,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, DbgLoc) {
@@ -362,9 +362,9 @@
 
   auto FiniCB = [&](InsertPointTy CodeGenIP) { ++NumFinalizationPoints; };
 
-  IRBuilder<>::InsertPoint AfterIP = OMPBuilder.CreateParallel(
-  Loc, BodyGenCB, PrivCB, FiniCB, nullptr, nullptr, OMP_PROC_BIND_default, false);
-
+  IRBuilder<>::InsertPoint AfterIP =
+  OMPBuilder.CreateParallel(Loc, BodyGenCB, PrivCB, FiniCB, nullptr,
+nullptr, OMP_PROC_BIND_default, false);
   EXPECT_EQ(NumBodiesGenerated, 1U);
   EXPECT_EQ(NumPrivatizedVars, 1U);
   EXPECT_EQ(NumFinalizationPoints, 1U);
@@ -372,10 +372,12 @@
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
 
+  OMPBuilder.finalize();
+
   EXPECT_NE(PrivAI, nullptr);
   Function *OutlinedFn = PrivAI->getFunction();
   EXPECT_NE(F, OutlinedFn);
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
   EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoUnwind));
   EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoRecurse));
   EXPECT_TRUE(OutlinedFn->hasParamAttribute(0, Attribute::NoAlias));
@@ -470,6 +472,7 @@
 
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
+  OMPBuilder.finalize();
 
   EXPECT_NE(PrivAI, nullptr);
   Function *OutlinedFn = PrivAI->getFunction();
@@ -595,6 +598,7 @@
 
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
+  OMPBuilder.finalize();
 
   EXPECT_FALSE(verifyModule(*M, ()));
 
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -1405,11 +1405,7 @@
   DISubprogram *OldSP = OldFunc.getSubprogram();
   LLVMContext  = OldFunc.getContext();
 
-  // See llvm.org/PR44560, OpenMP passes an invalid subprogram to CodeExtractor.
-  bool NeedWorkaroundForOpenMPIRBuilderBug =
-  OldSP && OldSP->getRetainedNodes()->isTemporary();
-
-  if (!OldSP || NeedWorkaroundForOpenMPIRBuilderBug) {
+  if (!OldSP) {
 // Erase any debug info the new function contains.
 stripDebugInfo(NewFunc);
 // Make sure the old function doesn't contain any non-local metadata refs.
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -93,6 +93,55 @@
 
 void OpenMPIRBuilder::initialize() { initializeTypes(M); }
 
+void OpenMPIRBuilder::finalize() {
+  for (OutlineInfo  : OutlineInfos) {
+assert(!OI.Blocks.empty() &&
+   "Outlined regions should have at least a single block!");
+BasicBlock *RegEntryBB = OI.Blocks.front();
+Function *OuterFn = RegEntryBB->getParent();
+CodeExtractorAnalysisCache CEAC(*OuterFn);
+CodeExtractor Extractor(OI.Blocks, /* DominatorTree */ nullptr,
+/* AggregateArgs */ false,
+/* BlockFrequencyInfo */ nullptr,
+/* BranchProbabilityInfo */ nullptr,
+/* AssumptionCache */ nullptr,
+/* AllowVarArgs */ true,
+/* AllowAlloca */ true,
+/* Suffix */ ".omp_par");
+
+LLVM_DEBUG(dbgs() << "Before outlining: " << *OuterFn << "\n");
+
+Function *OutlinedFn = Extractor.extractCodeRegion(CEAC);
+
+LLVM_DEBUG(dbgs() << "After

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert added a comment.

In D74372#1870620 , @vsk wrote:

> Thanks :). IIRC `check-clang` is enough to exercise the relevant code path, 
> as we get an assert in CodeExtractor without the workaround. (side note: 
> please don't take my comments here as blocking, I just wanted to see if we 
> could delete some cruft)


Can you verify I deleted the workaround? I was not sure if the workaround is 
all of it or only the OMP... part.
I run the test you mentioned w/ and w/o the debug flag you mentioned in the PR, 
both times it passes w/o assertion:

  PASS: Clang :: OpenMP/parallel_codegen.cpp 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert updated this revision to Diff 244002.
jdoerfert marked 6 inline comments as done.
jdoerfert added a comment.

Removed OMPIRBuilder workaround in extractor and addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -96,7 +96,7 @@
   EXPECT_EQ(cast(Barrier)->getArgOperand(1), GTID);
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancel) {
@@ -151,7 +151,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancelIfCond) {
@@ -212,7 +212,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancelBarrier) {
@@ -267,7 +267,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, DbgLoc) {
@@ -362,9 +362,9 @@
 
   auto FiniCB = [&](InsertPointTy CodeGenIP) { ++NumFinalizationPoints; };
 
-  IRBuilder<>::InsertPoint AfterIP = OMPBuilder.CreateParallel(
-  Loc, BodyGenCB, PrivCB, FiniCB, nullptr, nullptr, OMP_PROC_BIND_default, false);
-
+  IRBuilder<>::InsertPoint AfterIP =
+  OMPBuilder.CreateParallel(Loc, BodyGenCB, PrivCB, FiniCB, nullptr,
+nullptr, OMP_PROC_BIND_default, false);
   EXPECT_EQ(NumBodiesGenerated, 1U);
   EXPECT_EQ(NumPrivatizedVars, 1U);
   EXPECT_EQ(NumFinalizationPoints, 1U);
@@ -372,10 +372,12 @@
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
 
+  OMPBuilder.finalize();
+
   EXPECT_NE(PrivAI, nullptr);
   Function *OutlinedFn = PrivAI->getFunction();
   EXPECT_NE(F, OutlinedFn);
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
   EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoUnwind));
   EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoRecurse));
   EXPECT_TRUE(OutlinedFn->hasParamAttribute(0, Attribute::NoAlias));
@@ -470,6 +472,7 @@
 
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
+  OMPBuilder.finalize();
 
   EXPECT_NE(PrivAI, nullptr);
   Function *OutlinedFn = PrivAI->getFunction();
@@ -595,6 +598,7 @@
 
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
+  OMPBuilder.finalize();
 
   EXPECT_FALSE(verifyModule(*M, ()));
 
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -1405,11 +1405,7 @@
   DISubprogram *OldSP = OldFunc.getSubprogram();
   LLVMContext  = OldFunc.getContext();
 
-  // See llvm.org/PR44560, OpenMP passes an invalid subprogram to CodeExtractor.
-  bool NeedWorkaroundForOpenMPIRBuilderBug =
-  OldSP && OldSP->getRetainedNodes()->isTemporary();
-
-  if (!OldSP || NeedWorkaroundForOpenMPIRBuilderBug) {
+  if (!OldSP) {
 // Erase any debug info the new function contains.
 stripDebugInfo(NewFunc);
 // Make sure the old function doesn't contain any non-local metadata refs.
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -93,6 +93,55 @@
 
 void OpenMPIRBuilder::initialize() { initializeTypes(M); }
 
+void OpenMPIRBuilder::finalize() {
+  for (OutlineInfo  : OutlineInfos) {
+assert(!OI.Blocks.empty() &&
+   "Outlined regions should have at least a single block!");
+BasicBlock *RegEntryBB = OI.Blocks.front();
+Function *OuterFn = RegEntryBB->getParent();
+CodeExtractorAnalysisCache CEAC(*OuterFn);
+CodeExtractor Extractor(OI.Blocks, /* DominatorTree */ nullptr,
+/* AggregateArgs */ false,
+/* BlockFrequencyInfo */ nullptr,
+/* BranchProbabilityInfo */ nullptr,
+/* AssumptionCache */ nullptr,
+/* AllowVarArgs */ true,
+/* AllowAlloca */ true,
+/* Suffix */ ".omp_par");
+
+LLVM_DEBUG(dbgs() << "Before outlining: " << *OuterFn << "\n");
+
+

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Fady Ghanim via Phabricator via cfe-commits 
fghanim added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113
+  if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder())
+OMPBuilder->finalize();
 }

jdoerfert wrote:
> fghanim wrote:
> > jdoerfert wrote:
> > > fghanim wrote:
> > > > Does this mean that `finalize()` is being called twice everytime, here 
> > > > and `~OpenMPIRBuilder()`?
> > > > if yes, is there a reason why you want that?
> > > It's called twice now, not that it matters. The basic rule is, call it at 
> > > least once before you finalize the module. I'll remove the destructor for 
> > > now to make it explicit only.
> > Right now it doesn't matter, I agree. However, if later `finalize()` is 
> > modified for any reason, I worry this may introduce problems 
> > unintentionally.
> > 
> > In any case, if you decide to keep only one of them, I prefer to keep the 
> > one in `~OpenMPIRBuilder()`. This way the `OMPBuilder` is self contained, 
> > so regardless of the frontend using it, we can be certain it's always 
> > called at the end. You can indicate explicitly in the description comment 
> > for `finalize()` that it is being called in the destructor, if you want.
> That doesn't work, otherwise I would not call finalize here. At least it 
> doesn't work without extra changes. As it is right now, the OMPBuilder in 
> CodeGenModule is deleted *after* the IR is generated and emitted, so any 
> changes at destructor time are too late.
Got it. Then a minor suggestion is to just put a note in `finalize()` 
description about whatever you decide to do (i.e. keep both, remove the one in 
destructor, or something else entirely). This way, whoever is using/modifying 
the code is aware of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Vedant Kumar via Phabricator via cfe-commits 
vsk added a comment.

Thanks :). IIRC `check-clang` is enough to exercise the relevant code path, as 
we get an assert in CodeExtractor without the workaround. (side note: please 
don't take my comments here as blocking, I just wanted to see if we could 
delete some cruft)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert added a comment.

In D74372#1870547 , @vsk wrote:

> Can this delete `NeedWorkaroundForOpenMPIRBuilderBug` from 
> llvm/lib/Transforms/Utils/CodeExtractor.cpp?


I didn't know about the workaround but that was the plan. I'll verify and add 
it to the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Vedant Kumar via Phabricator via cfe-commits 
vsk added a comment.

Can this delete `NeedWorkaroundForOpenMPIRBuilderBug` from 
llvm/lib/Transforms/Utils/CodeExtractor.cpp?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert marked 2 inline comments as done.
jdoerfert added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113
+  if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder())
+OMPBuilder->finalize();
 }

fghanim wrote:
> jdoerfert wrote:
> > fghanim wrote:
> > > Does this mean that `finalize()` is being called twice everytime, here 
> > > and `~OpenMPIRBuilder()`?
> > > if yes, is there a reason why you want that?
> > It's called twice now, not that it matters. The basic rule is, call it at 
> > least once before you finalize the module. I'll remove the destructor for 
> > now to make it explicit only.
> Right now it doesn't matter, I agree. However, if later `finalize()` is 
> modified for any reason, I worry this may introduce problems unintentionally.
> 
> In any case, if you decide to keep only one of them, I prefer to keep the one 
> in `~OpenMPIRBuilder()`. This way the `OMPBuilder` is self contained, so 
> regardless of the frontend using it, we can be certain it's always called at 
> the end. You can indicate explicitly in the description comment for 
> `finalize()` that it is being called in the destructor, if you want.
That doesn't work, otherwise I would not call finalize here. At least it 
doesn't work without extra changes. As it is right now, the OMPBuilder in 
CodeGenModule is deleted *after* the IR is generated and emitted, so any 
changes at destructor time are too late.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Fady Ghanim via Phabricator via cfe-commits 
fghanim added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113
+  if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder())
+OMPBuilder->finalize();
 }

jdoerfert wrote:
> fghanim wrote:
> > Does this mean that `finalize()` is being called twice everytime, here and 
> > `~OpenMPIRBuilder()`?
> > if yes, is there a reason why you want that?
> It's called twice now, not that it matters. The basic rule is, call it at 
> least once before you finalize the module. I'll remove the destructor for now 
> to make it explicit only.
Right now it doesn't matter, I agree. However, if later `finalize()` is 
modified for any reason, I worry this may introduce problems unintentionally.

In any case, if you decide to keep only one of them, I prefer to keep the one 
in `~OpenMPIRBuilder()`. This way the `OMPBuilder` is self contained, so 
regardless of the frontend using it, we can be certain it's always called at 
the end. You can indicate explicitly in the description comment for 
`finalize()` that it is being called in the destructor, if you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert marked 7 inline comments as done.
jdoerfert added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113
+  if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder())
+OMPBuilder->finalize();
 }

fghanim wrote:
> Does this mean that `finalize()` is being called twice everytime, here and 
> `~OpenMPIRBuilder()`?
> if yes, is there a reason why you want that?
It's called twice now, not that it matters. The basic rule is, call it at least 
once before you finalize the module. I'll remove the destructor for now to make 
it explicit only.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:275
+  /// Get a handle for a new region that will be outlined later.
+  OutlineInfo () {
+OutlineInfos.push_back({});

JonChesterfield wrote:
> Calling getNewOutlineInfo will invalidate the references previously returned. 
> That's not a bug in this patch but looks like it'll be easy to get wrong in 
> future.
> 
> Perhaps the backing store should be a linked list, such that push_back 
> doesn't invalidate any existing references?
The address of these objects is not stable but without that requirement we 
don't need a list. We make this function take a `OutlineInfo` object that we 
than move into the vector if that is preferred.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:114
+
+// Add some known attributes to the outlined function.
+Function *OutlinedFn = Extractor.extractCodeRegion(CEAC);

rogfer01 wrote:
> This comment seems misplaced now.
Agreed, will be moved.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:587
+  CallInst *CI = cast(OutlinedFn.user_back());
+  CI->getParent()->setName("omp_parallel");
+  Builder.SetInsertPoint(CI);

JonChesterfield wrote:
> s/parallel/outlined?
this is in the `omp parallel` callback so "parallel" is still fine. 



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:654-655
+Instruction *PreFiniTI = PRegPreFiniBB->getTerminator();
+OuterFn->dump();
+PreFiniTI->dump();
+assert(PreFiniTI->getNumSuccessors() == 1 &&

rogfer01 wrote:
> I think you forgot to wrap these in a `LLVM_DEBUG`
I simplify forgot the remove them. Thx for noticing.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:663
 
-  return AfterIP;
+InsertPointTy ExitIP(UI->getParent(), UI->getParent()->end());
+UI->eraseFromParent();

rogfer01 wrote:
> This used to be called `AfterIP`. Probably the exact name is not that 
> important but the unit tests still use `AfterIP` so I'd suggest to keep both 
> names in sync.
I'll call it `AfterIP` again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Fady Ghanim via Phabricator via cfe-commits 
fghanim added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:113
+  if (llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder())
+OMPBuilder->finalize();
 }

Does this mean that `finalize()` is being called twice everytime, here and 
`~OpenMPIRBuilder()`?
if yes, is there a reason why you want that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-11 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added a comment.

Patch looks good with the above nits.

I'm not totally sure about the callback vs running a separate IR pass after the 
finalize() call, but when the callback is this simple it looks fine. I like 
that this preserves the current semantics.




Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:275
+  /// Get a handle for a new region that will be outlined later.
+  OutlineInfo () {
+OutlineInfos.push_back({});

Calling getNewOutlineInfo will invalidate the references previously returned. 
That's not a bug in this patch but looks like it'll be easy to get wrong in 
future.

Perhaps the backing store should be a linked list, such that push_back doesn't 
invalidate any existing references?



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:587
+  CallInst *CI = cast(OutlinedFn.user_back());
+  CI->getParent()->setName("omp_parallel");
+  Builder.SetInsertPoint(CI);

s/parallel/outlined?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-10 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits 
rogfer01 added inline comments.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:114
+
+// Add some known attributes to the outlined function.
+Function *OutlinedFn = Extractor.extractCodeRegion(CEAC);

This comment seems misplaced now.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:654-655
+Instruction *PreFiniTI = PRegPreFiniBB->getTerminator();
+OuterFn->dump();
+PreFiniTI->dump();
+assert(PreFiniTI->getNumSuccessors() == 1 &&

I think you forgot to wrap these in a `LLVM_DEBUG`



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:663
 
-  return AfterIP;
+InsertPointTy ExitIP(UI->getParent(), UI->getParent()->end());
+UI->eraseFromParent();

This used to be called `AfterIP`. Probably the exact name is not that important 
but the unit tests still use `AfterIP` so I'd suggest to keep both names in 
sync.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

2020-02-10 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert created this revision.
jdoerfert added reviewers: rogfer01, ABataev, JonChesterfield, 
kiranchandramohan, fghanim.
Herald added subscribers: cfe-commits, guansong, bollu, hiraditya.
Herald added projects: clang, LLVM.

In order to fix PR44560 and to prepare for loop transformations we now
finalize a function late, which will also do the outlining late. The
logic is as before but the actual outlining step happens now after the
function was fully constructed. Once we have loop transformations we
can apply them in the finalize step before the outlining.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74372

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -96,7 +96,7 @@
   EXPECT_EQ(cast(Barrier)->getArgOperand(1), GTID);
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancel) {
@@ -151,7 +151,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancelIfCond) {
@@ -212,7 +212,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateCancelBarrier) {
@@ -267,7 +267,7 @@
   OMPBuilder.popFinalizationCB();
 
   Builder.CreateUnreachable();
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
 }
 
 TEST_F(OpenMPIRBuilderTest, DbgLoc) {
@@ -362,9 +362,9 @@
 
   auto FiniCB = [&](InsertPointTy CodeGenIP) { ++NumFinalizationPoints; };
 
-  IRBuilder<>::InsertPoint AfterIP = OMPBuilder.CreateParallel(
-  Loc, BodyGenCB, PrivCB, FiniCB, nullptr, nullptr, OMP_PROC_BIND_default, false);
-
+  IRBuilder<>::InsertPoint AfterIP =
+  OMPBuilder.CreateParallel(Loc, BodyGenCB, PrivCB, FiniCB, nullptr,
+nullptr, OMP_PROC_BIND_default, false);
   EXPECT_EQ(NumBodiesGenerated, 1U);
   EXPECT_EQ(NumPrivatizedVars, 1U);
   EXPECT_EQ(NumFinalizationPoints, 1U);
@@ -372,10 +372,12 @@
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
 
+  OMPBuilder.finalize();
+
   EXPECT_NE(PrivAI, nullptr);
   Function *OutlinedFn = PrivAI->getFunction();
   EXPECT_NE(F, OutlinedFn);
-  EXPECT_FALSE(verifyModule(*M));
+  EXPECT_FALSE(verifyModule(*M, ()));
   EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoUnwind));
   EXPECT_TRUE(OutlinedFn->hasFnAttribute(Attribute::NoRecurse));
   EXPECT_TRUE(OutlinedFn->hasParamAttribute(0, Attribute::NoAlias));
@@ -470,6 +472,7 @@
 
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
+  OMPBuilder.finalize();
 
   EXPECT_NE(PrivAI, nullptr);
   Function *OutlinedFn = PrivAI->getFunction();
@@ -595,6 +598,7 @@
 
   Builder.restoreIP(AfterIP);
   Builder.CreateRetVoid();
+  OMPBuilder.finalize();
 
   EXPECT_FALSE(verifyModule(*M, ()));
 
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -93,6 +93,56 @@
 
 void OpenMPIRBuilder::initialize() { initializeTypes(M); }
 
+void OpenMPIRBuilder::finalize() {
+  for (OutlineInfo  : OutlineInfos) {
+assert(!OI.Blocks.empty() &&
+   "Outlined regions should have at least a single block!");
+BasicBlock *RegEntryBB = OI.Blocks.front();
+Function *OuterFn = RegEntryBB->getParent();
+CodeExtractorAnalysisCache CEAC(*OuterFn);
+CodeExtractor Extractor(OI.Blocks, /* DominatorTree */ nullptr,
+/* AggregateArgs */ false,
+/* BlockFrequencyInfo */ nullptr,
+/* BranchProbabilityInfo */ nullptr,
+/* AssumptionCache */ nullptr,
+/* AllowVarArgs */ true,
+/* AllowAlloca */ true,
+/* Suffix */ ".omp_par");
+
+LLVM_DEBUG(dbgs() << "Before outlining: " << *OuterFn << "\n");
+
+// Add some known attributes to the outlined function.
+Function *OutlinedFn = Extractor.extractCodeRegion(CEAC);
+
+LLVM_DEBUG(dbgs() << "After  outlining: " << *OuterFn << "\n");
+LLVM_DEBUG(dbgs() << "   Outlined function: " << *OutlinedFn << "\n");
+
+// For compability with the clang CG we move the outlined function after the
+// one with the parallel region.
+OutlinedFn->removeFromParent();
+M.getFunctionList().insertAfter(OuterFn->getIterator(),