[PATCH] D33411: [Polly] Add handling of Top Level Regions

[Lukas Böhm via Phabricator via llvm-commits]

lksbhm created this revision.
lksbhm added a project: Polly.
Herald added a subscriber: bollu.

My goal is to make the newly added `AllowWholeFunctions` options more usable/powerful.

The changes to ScopBuilder.cpp are exclusively checks to prevent `Region.getExit()` from being dereferenced, since Top Level Regions (TLRs) don't have an exit block.

In ScopDetection's `isValidCFG`, I removed a check that disallowed ReturnInstructions to have return values. This might of course have been intentional, so I would welcome your feedback on this and maybe a small explanation why return values are forbidden. Maybe it can be done but needs more changes elsewhere?

The remaining changes in ScopDetection are simply to consider the AllowWholeFunctions option in more places, i.e. allow TLRs when it is set and once again avoid derefererncing `getExit()` if it doesn't exist.

Finally, in ScopHelper.cpp I extended `polly::isErrorBlock` to handle regions without exit blocks as well: The original check was if a given BasicBlock dominates all predecessors of the exit block. Therefore I do the same for TLRs by regarding all BasicBlocks terminating with a ReturnInst as predecessors of a "virtual" function exit block.


Repository:
  rL LLVM

https://reviews.llvm.org/D33411

Files:
  lib/Analysis/ScopBuilder.cpp
  lib/Analysis/ScopDetection.cpp
  lib/Support/ScopHelper.cpp


Index: lib/Support/ScopHelper.cpp
===================================================================
--- lib/Support/ScopHelper.cpp
+++ lib/Support/ScopHelper.cpp
@@ -380,9 +380,15 @@
   // Basic blocks that are always executed are not considered error blocks,
   // as their execution can not be a rare event.
   bool DominatesAllPredecessors = true;
-  for (auto Pred : predecessors(R.getExit()))
-    if (R.contains(Pred) && !DT.dominates(&BB, Pred))
-      DominatesAllPredecessors = false;
+  if (R.isTopLevelRegion()) {
+    for (BasicBlock &I : *R.getEntry()->getParent())
+      if (isa<ReturnInst>(I.getTerminator()) && !DT.dominates(&BB, &I))
+        DominatesAllPredecessors = false;
+  } else {
+    for (auto Pred : predecessors(R.getExit()))
+      if (R.contains(Pred) && !DT.dominates(&BB, Pred))
+        DominatesAllPredecessors = false;
+  }
 
   if (DominatesAllPredecessors)
     return false;
Index: lib/Analysis/ScopDetection.cpp
===================================================================
--- lib/Analysis/ScopDetection.cpp
+++ lib/Analysis/ScopDetection.cpp
@@ -578,7 +578,7 @@
     return true;
 
   // Return instructions are only valid if the region is the top level region.
-  if (isa<ReturnInst>(TI) && !CurRegion.getExit() && TI->getNumOperands() == 0)
+  if (isa<ReturnInst>(TI) && CurRegion.isTopLevelRegion())
     return true;
 
   Value *Condition = getConditionFromTerminator(TI);
@@ -1485,13 +1485,14 @@
 
   DEBUG(dbgs() << "Checking region: " << CurRegion.getNameStr() << "\n\t");
 
-  if (CurRegion.isTopLevelRegion()) {
+  if (!AllowFullFunction && CurRegion.isTopLevelRegion()) {
     DEBUG(dbgs() << "Top level region is invalid\n");
     return false;
   }
 
   DebugLoc DbgLoc;
-  if (isa<UnreachableInst>(CurRegion.getExit()->getTerminator())) {
+  if (CurRegion.getExit() &&
+      isa<UnreachableInst>(CurRegion.getExit()->getTerminator())) {
     DEBUG(dbgs() << "Unreachable in exit\n");
     return invalid<ReportUnreachableInExit>(Context, /*Assert=*/true,
                                             CurRegion.getExit(), DbgLoc);
Index: lib/Analysis/ScopBuilder.cpp
===================================================================
--- lib/Analysis/ScopBuilder.cpp
+++ lib/Analysis/ScopBuilder.cpp
@@ -894,12 +894,14 @@
     return;
 
   // PHINodes in the SCoP region's exit block are also uses to be checked.
-  for (auto &Inst : *S->getRegion().getExit()) {
-    if (!isa<PHINode>(Inst))
-      break;
+  if (!S->getRegion().isTopLevelRegion()) {
+    for (auto &Inst : *S->getRegion().getExit()) {
+      if (!isa<PHINode>(Inst))
+        break;
 
-    for (auto &Op : Inst.operands())
-      verifyUse(S, Op, LI);
+      for (auto &Op : Inst.operands())
+        verifyUse(S, Op, LI);
+    }
   }
 }
 #endif
@@ -917,7 +919,7 @@
   // To handle these PHI nodes later we will now model their operands as scalar
   // accesses. Note that we do not model anything in the exit block if we have
   // an exiting block in the region, as there will not be any splitting later.
-  if (!scop->hasSingleExitEdge())
+  if (!R.isTopLevelRegion() && !scop->hasSingleExitEdge())
     buildAccessFunctions(*R.getExit(), nullptr,
                          /* IsExitBlock */ true);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33411.99761.patch
Type: text/x-patch
Size: 3238 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170522/9d63b33e/attachment.bin>