[PATCH] D12051: Allow PHI nodes in the exit block of regions

[Johannes Doerfert via llvm-commits]

jdoerfert marked an inline comment as done.
jdoerfert added a comment.

I will push another version later.

The problematic part I encountered are PHI nodes that use values as operands. Here I tried to circumvent them by looking through the new PHI node in the exit and replacing it dynamically with the one that is now in the region [BlockGenerators.cpp:529]. However, I don't like this solution so I came up with a different one I will push here.


================
Comment at: lib/Analysis/TempScopInfo.cpp:114
@@ -112,1 +113,3 @@
+  // In this case we model the operands but not the PHI itself.
+  if (PHI->getParent() != R.getExit() && canSynthesize(PHI, LI, SE, &R))
     return;
----------------
Meinersbur wrote:
> What happens otherwise?
The PHI operands are not modeled, hence never stored into the escape location.

================
Comment at: lib/Analysis/TempScopInfo.cpp:470
@@ +469,3 @@
+    NumInRegion += R.contains(PredBB);
+  if (NumInRegion > 1)
+    buildAccessFunctions(R, *ExitBB);
----------------
Meinersbur wrote:
> if (!R.getExitingBlock())
True.

================
Comment at: lib/CodeGen/BlockGenerators.cpp:532
@@ +531,3 @@
+        BasePHI = ExitBlockPHI;
+      }
+
----------------
Meinersbur wrote:
> Is this necessary? Can't one just use the original PHI to represent the virtual memory location? It's not directly used anyway.
I don't think so as it is linked via the escape map to the rewiring at the end.

================
Comment at: lib/CodeGen/BlockGenerators.cpp:1202
@@ -1151,1 +1201,3 @@
+    if (ExitBlockPHI)
+      handleOutsideUsers(R, ExitBlockPHI, nullptr, PHIOpMap);
   }
----------------
Meinersbur wrote:
> The last 30 lines are repetative. Refactor?
That might be an idea for a different patch though.


http://reviews.llvm.org/D12051