[polly] r217508 - [Refactor] Cleanup isl code generation

[Johannes Doerfert]

On 09/15, Tobias Grosser wrote:
> On 14/09/2014 18:45, Johannes Doerfert wrote:
> >Attached a fix for the problem. Could you take a look?
> 
> Thanks.
> 
> > From ee01feec4f9b7616f4a37fad7c71bd4f4d471d6e Mon Sep 17 00:00:00 2001
> >From: Johannes Doerfert<doerfert at cs.uni-saarland.de>
> >Date: Sun, 14 Sep 2014 18:41:57 +0200
> >Subject: [PATCH] [Fix] Rewire the Region after a unconditional entry edge is
> >  created
> >
> >   We use SplitEdge to split a conditional entry edge of the SCoP region.
> >   However, SplitEdge can cause two different situations. This patch tests
> >   which one is present and deals with the former unhandled one.
> 
> It would be interesting to mention the actual issue in the commit message.
I thought I did. The second, before not-handled way, SplitEdge can "split
an edge".

> At a first look I thought we missed the update of the ScopStmt, but now
> looking again it seems as if we missed the simple case and incorrectly
> updated the ScoP. Is this right?
No. Only the simple case was covered, thus the one were the SCoP doesn't
need to be updated. Now both situations are implemented, the simple one
(then branch) and the complicated one where we update the SCoP. I just
moved this update mechanism into an own function as it is used twice now.

> Also, it would be good to have a test case that reproduces the problem.
Yes, I will try to create one.

> Cheers,
> Tobias
> 
> 
> >  lib/Support/ScopHelper.cpp | 52 ++++++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 43 insertions(+), 9 deletions(-)
> >
> >diff --git a/lib/Support/ScopHelper.cpp b/lib/Support/ScopHelper.cpp
> >index 01c1dd0..0e2f6fa 100644
> >--- a/lib/Support/ScopHelper.cpp
> >+++ b/lib/Support/ScopHelper.cpp
> >@@ -86,6 +86,17 @@ BasicBlock *polly::createSingleExitEdge(Region *R, Pass *P) {
> >    return SplitBlockPredecessors(BB, Preds, ".region", P);
> >  }
> >
> >+static void replaceScopAndRegionEntry(polly::Scop *S, BasicBlock *OldEntry,
> >+                                      BasicBlock *NewEntry) {
> >+  for (polly::ScopStmt *Stmt : *S)
> >+    if (Stmt->getBasicBlock() == OldEntry) {
> >+      Stmt->setBasicBlock(NewEntry);
> >+      break;
> >+    }
> >+
> >+  S->getRegion().replaceEntryRecursive(NewEntry);
> >+}
> >+
> >  BasicBlock *polly::simplifyRegion(Scop *S, Pass *P) {
> >    Region *R = &S->getRegion();
> >
> >@@ -96,20 +107,43 @@ BasicBlock *polly::simplifyRegion(Scop *S, Pass *P) {
> >    if (!EnteringBB) {
> >      BasicBlock *OldEntry = R->getEntry();
> >      BasicBlock *NewEntry = SplitBlock(OldEntry, OldEntry->begin(), P);
> >-
> >-    for (ScopStmt *Stmt : *S)
> >-      if (Stmt->getBasicBlock() == OldEntry) {
> >-        Stmt->setBasicBlock(NewEntry);
> >-        break;
> >-      }
> >-
> >-    R->replaceEntryRecursive(NewEntry);
> >+    replaceScopAndRegionEntry(S, OldEntry, NewEntry);
> >      EnteringBB = OldEntry;
> >    }
> >
> >    // Create an unconditional entry edge.
> >    if (EnteringBB->getTerminator()->getNumSuccessors() != 1) {
> >-    EnteringBB = SplitEdge(EnteringBB, R->getEntry(), P);
> >+    BasicBlock *EntryBB = R->getEntry();
> >+    BasicBlock *SplitEdgeBB = SplitEdge(EnteringBB, EntryBB, P);
> >+
> >+    // Once the edge between EnteringBB and EntryBB is split, two cases arise.
> >+    // The first is simple. The new block is inserted between EnteringBB and
> >+    // EntryBB. In this case no further action is needed. However it might
> >+    // happen that the new block is inserted __after__ EntryBB causing the
> >+    // following situation:
> >+    //
> >+    // EnteringBB
> >+    //     |
> >+    //    / \
> >+    //    |  \-> some_other_BB_not_in_R
> >+    //    V
> >+    // EntryBB
> >+    //    |
> >+    //    V
> >+    // SplitEdgeBB
> >+    //
> >+    // In this case we need to swap the role of EntryBB and SplitEdgeBB.
> >+
> >+    // Check which case SplitEdge produced:
> >+    if (SplitEdgeBB->getTerminator()->getSuccessor(0) == EntryBB) {
> >+      // First (simple) case.
> >+      EnteringBB = SplitEdgeBB;
> >+    } else {
> >+      // Second (complicated) case.
> >+      replaceScopAndRegionEntry(S, EntryBB, SplitEdgeBB);
> >+      EnteringBB = EntryBB;
> >+    }
> >+
> >      EnteringBB->setName("polly.entering.block");
> >    }
> >
> >-- 2.1.0
> >
> >
> >
> >_______________________________________________
> >llvm-commits mailing list
> >llvm-commits at cs.uiuc.edu
> 

-- 

Johannes Doerfert
Researcher / PhD Student

Compiler Design Lab (Prof. Hack)
Saarland University, Computer Science
Building E1.3, Room 4.26

Tel. +49 (0)681 302-57521 : doerfert at cs.uni-saarland.de
Fax. +49 (0)681 302-3065  : http://www.cdl.uni-saarland.de/people/doerfert
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 213 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140915/fe9e701e/attachment.sig>