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

[Tobias Grosser]

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. 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?

Also, it would be good to have a test case that reproduces the problem.

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