[llvm] r353911 - [LoopSimplifyCFG] Re-enable const branch folding by default

Roman Tereshin via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 20 23:40:46 PST 2019


Hi Max,

Thank you for replying so quickly!

I’ve applied the change locally and the original IR module I used to bugpoint the reproducer I’ve sent you earlier failed anyway, this time for a different reason.
I can’t send the module as it is for IP reasons, but here’s another bugpoint from the same source (with some manual tidying up):

define void @main(i32* %a, i32* %b) {
entry:
  br label %for.body

for.body:
  %i = phi i32 [ 0, %entry ], [ %i.inc, %latch ]
  br label %switch.bb

switch.bb:
  switch i2 1, label %default [
    i2 1, label %case
  ]

case:
  br label %latch

default:
  unreachable

latch:
  store i32 %i, i32* %a
  store i32 %i, i32* %b
  %i.inc = add nsw i32 %i, 1
  %exitcond = icmp eq i32 %i.inc, 4
  br i1 %exitcond, label %exit, label %for.body

exit:
  ret void
}

./bin/opt -loop-simplifycfg loop-simplifycfg-term-folding-AND-mssa-loop-dependency-2.ll -enable-mssa-loop-dependency=true -verify-memoryssa=true -enable-loop-simplifycfg-term-folding=true -S -o -
Assertion failed: (dominates(MD, U) && "Memory Def does not dominate it's uses"), function verifyDomination, file ../lib/Analysis/MemorySSA.cpp, line 1912.

If I don’t apply https://reviews.llvm.org/rL354547 <https://reviews.llvm.org/rL354547> it fails the same way the previous reproducer did, so I guess this test is strictly better. Sorry for overbugpointing the first time around!

Thanks,
Roman



> On Feb 20, 2019, at 10:12 PM, Alina Sbirlea <alina.sbirlea at gmail.com> wrote:
> 
> Are you sure your update is complete?
> It should theoretically match the DT update, i.e. include insert edges not just the delete. All updates should be done together using the same list of updates used for DT, and an up to date DT (see MSSAU->applyUpdates(DTUpdates, DT)).
> 
> On Wed, Feb 20, 2019 at 9:53 PM Maxim Kazantsev via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> Should be fixed by https://reviews.llvm.org/rL354547 <https://reviews.llvm.org/rL354547>
>  
> 
> From: Maxim Kazantsev 
> Sent: Thursday, February 21, 2019 11:49 AM
> To: 'rtereshin at apple.com <mailto:rtereshin at apple.com>' <rtereshin at apple.com <mailto:rtereshin at apple.com>>
> Cc: llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>>; dberlin at dberlin.org <mailto:dberlin at dberlin.org>; asbirlea at google.com <mailto:asbirlea at google.com>
> Subject: RE: [llvm] r353911 - [LoopSimplifyCFG] Re-enable const branch folding by default
> 
>  
> 
> Hi Roman,
> 
>  
> 
> Thanks for notification! I’ll fix it asap.
> 
>  
> 
> --Max
> 
>  
> 
> From: rtereshin at apple.com <mailto:rtereshin at apple.com> <rtereshin at apple.com <mailto:rtereshin at apple.com>> 
> Sent: Thursday, February 21, 2019 10:25 AM
> To: Maxim Kazantsev <max.kazantsev at azul.com <mailto:max.kazantsev at azul.com>>
> Cc: llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>>; dberlin at dberlin.org <mailto:dberlin at dberlin.org>; asbirlea at google.com <mailto:asbirlea at google.com>
> Subject: Re: [llvm] r353911 - [LoopSimplifyCFG] Re-enable const branch folding by default
> 
>  
> 
> Hi Max,
> 
> Looks like this doesn’t collaborate with MemorySSA very well:
> 
> ./bin/opt -loop-simplifycfg loop-simplifycfg-term-folding-AND-mssa-loop-dependency.ll -enable-mssa-loop-dependency=true -verify-memoryssa=true -enable-loop-simplifycfg-term-folding=true -S -o -
> Assertion failed: (find(predecessors(&B), Phi->getIncomingBlock(I)) != pred_end(&B) && "Incoming phi block not a block predecessor"), function verifyDefUses, file ../lib/Analysis/MemorySSA.cpp, line 1947.
> 
> The IR is:
> 
> define void @main() {
> entry:
>   br label %for.body
> 
> for.body:                                         ; preds = %exit, %entry
>   br label %switch.bb <http://switch.bb/>
> 
> switch.bb <http://switch.bb/>:                                        ; preds = %for.body
>   switch i2 1, label %default.bb <http://default.bb/> [
>     i2 1, label %case.bb <http://case.bb/>
>   ]
> 
> case.bb <http://case.bb/>:                                          ; preds = %switch
>   br label %exit
> 
> default.bb <http://default.bb/>:                                       ; preds = %switch
>   unreachable
> 
> exit:                                             ; preds = %case.bb <http://case.bb/>
>   call void @foo()
>   br label %for.body
> }
> 
> declare void @foo()
> 
> 
> Thanks,
> Roman
> 
> 
> 
> > On Feb 12, 2019, at 10:12 PM, Max Kazantsev via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> > 
> > Author: mkazantsev
> > Date: Tue Feb 12 22:12:48 2019
> > New Revision: 353911
> > 
> > URL: http://llvm.org/viewvc/llvm-project?rev=353911&view=rev <http://llvm.org/viewvc/llvm-project?rev=353911&view=rev>
> > Log:
> > [LoopSimplifyCFG] Re-enable const branch folding by default
> > 
> > Known underlying bugs have been fixed, intensive fuzz testing did not
> > find any new problems. Re-enabling by default. Feel free to revert if
> > it causes any functional failures.
> > 
> > Modified:
> >    llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
> > 
> > Modified: llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp?rev=353911&r1=353910&r2=353911&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp?rev=353911&r1=353910&r2=353911&view=diff>
> > ==============================================================================
> > --- llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp Tue Feb 12 22:12:48 2019
> > @@ -41,7 +41,7 @@ using namespace llvm;
> > #define DEBUG_TYPE "loop-simplifycfg"
> > 
> > static cl::opt<bool> EnableTermFolding("enable-loop-simplifycfg-term-folding",
> > -                                       cl::init(false));
> > +                                       cl::init(true));
> > 
> > STATISTIC(NumTerminatorsFolded,
> >           "Number of terminators folded to unconditional branches");
> > 
> > 
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>_______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190220/f8dc9bba/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: loop-simplifycfg-term-folding-AND-mssa-loop-dependency-2.ll
Type: application/octet-stream
Size: 456 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190220/f8dc9bba/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190220/f8dc9bba/attachment-0001.html>


More information about the llvm-commits mailing list