[llvm] r324572 - [SimplifyCFG] Re-apply Relax restriction for folding unconditional branches
David Green via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 13 10:06:49 PST 2018
Hello
Yes, I agree it's bit odd. I think the preheaders are recreated before they are needed, so it may just be code-ordering that's leading to the regressions. That's what I've seen so far at least, I've not managed to tell why the ordering leads to the differences yet. I'll keep looking.
But.. as far as I can tell the intent of this code is to preserve loop simple form, and if we are removing the preheader then we are not doing that. Any ideas how to sensibly prevent this from acting on preheaders? If we had loop info we could check if the BB is in the loop, without that I'm not sure how to tell the two types of blocks apart correctly. I don't think we have a domtree to check that way either.
Dave
From: Serguei Katkov <serguei.katkov at azul.com>
Sent: 13 February 2018 17:11
To: Serguei Katkov; David Green
Cc: nd; llvm-commits
Subject: RE: [llvm] r324572 - [SimplifyCFG] Re-apply Relax restriction for folding unconditional branches
Just to clarify (I'm not sure I was clear here :)).
1) The motivation was to remove BBs in a loop.
2) Removing of empty pre-header is a kind of side-effect.
3) It is interesting why removing of pre-headers results in perfromance regressions.
Thank you,
Serguei.
> -----Original Message-----
> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf
> Of Serguei Katkov via llvm-commits
> Sent: Wednesday, February 14, 2018 12:08 AM
> To: David Green <David.Green at arm.com>; llvm-commits <llvm-
> commits at lists.llvm.org>
> Cc: nd <nd at arm.com>
> Subject: RE: [llvm] r324572 - [SimplifyCFG] Re-apply Relax restriction for
> folding unconditional branches
>
> Hello David,
>
> the main idea was removing empty BB in a loop coming to header (latch) but
> not a removing of pre-header actually.
>
> However it is not clear why removing an empty pre-header comes to
> regressions. Jut interesting to know about the reason.
>
> Thank you,
> Serguei.
>
> > -----Original Message-----
> > From: David Green [mailto:David.Green at arm.com]
> > Sent: Tuesday, February 13, 2018 11:40 PM
> > To: Serguei Katkov <serguei.katkov at azul.com>; llvm-commits <llvm-
> > commits at lists.llvm.org>
> > Cc: nd <nd at arm.com>
> > Subject: Re: [llvm] r324572 - [SimplifyCFG] Re-apply Relax restriction
> > for folding unconditional branches
> >
> > Hello
> >
> > First up, sorry for the lateness of this response.
> >
> > Secondly, I am seeing a fair number of small to medium size
> > performance regressions from this patch, without many improvements (on
> > ARM). Is the idea that this should be removing preheaders? Or just empty
> blocks in loops?
> >
> > The only regressions I have analysed so far have turned into "code is
> > in a different order, the backend does something slightly different as a
> result".
> > And many regressions only appear to show up when LTO is enabled.
> >
> > So I don't have any sensible reproducers yet, but any thoughts about
> > it removing preheaders? It doesn't seem like that was the intent.
> > Dave
> >
> >
> >
> > From: llvm-commits <llvm-commits-bounces at lists.llvm.org> on behalf of
> > Serguei Katkov via llvm-commits <llvm-commits at lists.llvm.org>
> > Sent: 08 February 2018 07:16
> > To: llvm-commits at lists.llvm.org
> > Subject: [llvm] r324572 - [SimplifyCFG] Re-apply Relax restriction for
> > folding unconditional branches
> >
> > Author: skatkov
> > Date: Wed Feb 7 23:16:29 2018
> > New Revision: 324572
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=324572&view=rev
> > Log:
> > [SimplifyCFG] Re-apply Relax restriction for folding unconditional
> > branches
> >
> > The commit rL308422 introduces a restriction for folding unconditional
> > branches. Specifically if empty block with unconditional branch leads
> > to header of the loop then elimination of this basic block is prohibited.
> > However it seems this condition is redundantly strict.
> > If elimination of this basic block does not introduce more back edges
> > then we can eliminate this block.
> >
> > The patch implements this relax of restriction.
> >
> > The test profile/Linux/counter_promo_nest.c in compiler-rt project is
> > updated to meet this change.
> >
> > Reviewers: efriedma, mcrosier, pacxx, hsung, davidxl Reviewed By:
> > pacxx
> > Subscribers: llvm-commits
> > Differential Revision: https://reviews.llvm.org/D42691
> >
> > Added:
> > llvm/trunk/test/Transforms/SimplifyCFG/UncondBranchToHeader.ll
> > Modified:
> > llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
> > llvm/trunk/test/Transforms/LoopUnroll/peel-loop.ll
> > llvm/trunk/test/Transforms/LoopUnswitch/2015-06-17-Metadata.ll
> > llvm/trunk/test/Transforms/LoopUnswitch/infinite-loop.ll
> >
> > Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
> > URL: http://llvm.org/viewvc/llvm-
> > project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=324572&r1=
> > 32
> > 4571&r2=324572&view=diff
> >
> ==========================================================
> > ====================
> > --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Wed Feb 7
> > +++ 23:16:29
> > +++ 2018
> > @@ -5733,9 +5733,12 @@ bool SimplifyCFGOpt::SimplifyUncondBranc
> > // header. (This is for early invocations before loop simplify and
> > // vectorization to keep canonical loop forms for nested loops. These
> blocks
> > // can be eliminated when the pass is invoked later in the
> > back-end.)
> > + // Note that if BB has only one predecessor then we do not
> > + introduce new // backedge, so we can eliminate BB.
> > bool NeedCanonicalLoop =
> > Options.NeedCanonicalLoop &&
> > - (LoopHeaders && (LoopHeaders->count(BB) || LoopHeaders-
> > >count(Succ)));
> > + (LoopHeaders && std::distance(pred_begin(BB), pred_end(BB)) > 1
> &&
> > + (LoopHeaders->count(BB) || LoopHeaders->count(Succ)));
> > BasicBlock::iterator I = BB->getFirstNonPHIOrDbg()->getIterator();
> > if (I->isTerminator() && BB != &BB->getParent()->getEntryBlock() &&
> > !NeedCanonicalLoop &&
> > TryToSimplifyUncondBranchFromEmptyBlock(BB))
> >
> > Modified: llvm/trunk/test/Transforms/LoopUnroll/peel-loop.ll
> > URL: http://llvm.org/viewvc/llvm-
> > project/llvm/trunk/test/Transforms/LoopUnroll/peel-
> > loop.ll?rev=324572&r1=324571&r2=324572&view=diff
> >
> ==========================================================
> > ====================
> > --- llvm/trunk/test/Transforms/LoopUnroll/peel-loop.ll (original)
> > +++ llvm/trunk/test/Transforms/LoopUnroll/peel-loop.ll Wed Feb 7
> > +++ 23:16:29 2018
> > @@ -19,10 +19,8 @@
> > ; CHECK: store i32 2, i32* %[[INC2]], align 4 ; CHECK: %[[CMP3:.*]]
> > = icmp eq
> > i32 %k, 3 ; CHECK: br i1 %[[CMP3]], label %for.end, label
> > %[[LOOP_PH:.*]] -;
> > CHECK: [[LOOP_PH]]:
> > -; CHECK: br label %[[LOOP:.*]]
> > -; CHECK: [[LOOP]]:
> > -; CHECK: %[[IV:.*]] = phi i32 [ 3, %[[LOOP_PH]] ], [ {{.*}},
> > %[[LOOP]] ]
> > +; CHECK: for.end:
> > +; CHECK: ret void
> >
> > define void @basic(i32* %p, i32 %k) #0 {
> > entry:
> > @@ -68,11 +66,8 @@ for.end:
> > ; CHECK: store i32 2, i32* %[[INC2]], align 4 ; CHECK: %[[CMP3:.*]]
> > = icmp eq
> > i32 %k, 3 ; CHECK: br i1 %[[CMP3]], label %for.end, label
> > %[[LOOP_PH:.*]] -;
> > CHECK: [[LOOP_PH]]:
> > -; CHECK: br label %[[LOOP:.*]]
> > -; CHECK: [[LOOP]]:
> > -; CHECK: %[[IV:.*]] = phi i32 [ 3, %[[LOOP_PH]] ], [ %[[IV:.*]],
> > %[[LOOP]] ] -;
> > CHECK: %ret = phi i32 [ 0, %entry ], [ 1, %[[NEXT0]] ], [ 2,
> > %[[NEXT1]] ], [ 3, %[[NEXT2]] ], [ %[[IV]], %[[LOOP]] ]
> > +; CHECK: for.end:
> > +; CHECK: %ret = phi i32 [ 0, %entry ], [ 1, %[[NEXT0]] ], [ 2,
> > +%[[NEXT1]] ], [ 3, %[[NEXT2]] ], [ %inc, %for.body ]
> > ; CHECK: ret i32 %ret
> > define i32 @output(i32* %p, i32 %k) #0 {
> > entry:
> >
> > Modified: llvm/trunk/test/Transforms/LoopUnswitch/2015-06-17-
> > Metadata.ll
> > URL: http://llvm.org/viewvc/llvm-
> > project/llvm/trunk/test/Transforms/LoopUnswitch/2015-06-17-
> > Metadata.ll?rev=324572&r1=324571&r2=324572&view=diff
> >
> ==========================================================
> > ====================
> > --- llvm/trunk/test/Transforms/LoopUnswitch/2015-06-17-Metadata.ll
> > (original)
> > +++ llvm/trunk/test/Transforms/LoopUnswitch/2015-06-17-Metadata.ll
> Wed
> > +++ Feb 7 23:16:29 2018
> > @@ -16,7 +16,7 @@ for.body:
> > %cmp1 = icmp eq i32 %a, 12345
> > br i1 %cmp1, label %if.then, label %if.else, !prof !0 ; CHECK:
> > %cmp1 = icmp eq i32 %a, 12345 -; CHECK-NEXT: br i1 %cmp1, label
> > %for.body.preheader.split.us, label %for.body.preheader.split, !prof
> > !0
> > +; CHECK-NEXT: br i1 %cmp1, label %for.body.us, label %for.body, !prof
> > +!0
> > if.then: ; preds = %for.body
> > ; CHECK: for.body.us:
> > ; CHECK: add nsw i32 %{{.*}}, 123
> > @@ -53,7 +53,7 @@ entry:
> > br label %for.body
> > ;CHECK: entry:
> > ;CHECK-NEXT: %cmp1 = icmp eq i32 1, 2
> > -;CHECK-NEXT: br i1 %cmp1, label %entry.split, label
> > %for.cond.cleanup.split, !prof !1
> > +;CHECK-NEXT: br i1 %cmp1, label %for.body, label
> > +%for.cond.cleanup.split, !prof !1
> > ;CHECK: for.body:
> > for.body: ; preds = %for.inc, %entry
> > %inc.i = phi i32 [ 0, %entry ], [ %inc, %if.then ]
> >
> > Modified: llvm/trunk/test/Transforms/LoopUnswitch/infinite-loop.ll
> > URL: http://llvm.org/viewvc/llvm-
> > project/llvm/trunk/test/Transforms/LoopUnswitch/infinite-
> > loop.ll?rev=324572&r1=324571&r2=324572&view=diff
> >
> ==========================================================
> > ====================
> > --- llvm/trunk/test/Transforms/LoopUnswitch/infinite-loop.ll
> > (original)
> > +++ llvm/trunk/test/Transforms/LoopUnswitch/infinite-loop.ll Wed Feb
> > +++ 7
> > +++ 23:16:29 2018
> > @@ -16,7 +16,7 @@
> > ; CHECK-NEXT: br i1 %a, label %entry.split, label %abort0.split
> >
> > ; CHECK: entry.split:
> > -; CHECK-NEXT: br i1 %b, label %entry.split.split, label %abort1.split
> > +; CHECK-NEXT: br i1 %b, label %for.body, label %abort1.split
> >
> > ; CHECK: for.body:
> > ; CHECK-NEXT: br label %for.body
> >
> > Added:
> llvm/trunk/test/Transforms/SimplifyCFG/UncondBranchToHeader.ll
> > URL: http://llvm.org/viewvc/llvm-
> >
> project/llvm/trunk/test/Transforms/SimplifyCFG/UncondBranchToHeader.ll
> > ?rev=324572&view=auto
> >
> ==========================================================
> > ====================
> > --- llvm/trunk/test/Transforms/SimplifyCFG/UncondBranchToHeader.ll
> > (added)
> > +++ llvm/trunk/test/Transforms/SimplifyCFG/UncondBranchToHeader.ll
> > Wed
> > +++ Feb 7 23:16:29 2018
> > @@ -0,0 +1,18 @@
> > +; RUN: opt < %s -simplifycfg -S | FileCheck %s
> > +
> > +; Check that we can get rid of empty block leading to header ; if it
> > +does not introduce new edge.
> > +define i32 @test(i32 %c) {
> > +entry:
> > + br label %header
> > +header:
> > + %i = phi i32 [0, %entry], [%i.1, %backedge]
> > + %i.1 = add i32 %i, 1
> > + %cmp = icmp slt i32 %i.1, %c
> > + br i1 %cmp, label %backedge, label %exit ; CHECK-NOT: backedge:
> > +backedge:
> > + br label %header
> > +exit:
> > + ret i32 %i
> > +}
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list