[llvm] r264099 - Keep CodeGenPrepare from preserving the domtree.

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 22 17:13:14 PDT 2016


Hi,

> On Mar 22, 2016, at 4:35 PM, George Burgess IV <george.burgess.iv at gmail.com> wrote:
> 
> Hi!
> 
> FWIW, my understanding is that the domtree isn't used in or after CGP, so invalidating it isn't really a problem with the standard pass ordering.


Fair enough :).

> I'm not super familiar with the pass manager/CGP though, so I could definitely be wrong.
> 
> > Would it be easy to teach CGP how to update it?
> 
> I have no clue. Like said, not super familiar with these parts of LLVM :)
> 
> > We have the availableIf idiom or something for such cases and that would avoid invalidating the whole thing
> 
> But that's the new pass manager only, yeah?

I was thinking of this thing:

AnalysisType * llvm::Pass::getAnalysisIfAvailable 	(		)	const

I think it is also available with the legacy pass manager.

Anyhow does not seem we need to bother ;).

Thanks,
Q.

> I added a FIXME to make it selectively preserved in the future. If we can do that now, I'd be happy to swap to selectively preserving it (since it seems we already track whether or not we may have invalidated the domtree).
> 
> > I don’t actually what in CGP invalidates the DomTree
> 
> A number of places seem to do so; grep for `ModifiedDT` :)
> 
> The example I ran into was:
> 
> $ opt test/Transforms/CodeGenPrepare/dom-tree.ll -S -loop-unroll -codegenprepare -domtree -analyze -print-after-all
> #### Snip ####
> *** IR Dump After Loop-Closed SSA Form Pass ***
> ; Function Attrs: norecurse nounwind readnone
> define i32 @f(i32 %a) #0 {
> entry:
>   br label %for.body
> 
> for.cond.cleanup:                                 ; preds = %for.body
>   %or.lcssa = phi i32 [ %or, %for.body ]
>   ret i32 %or.lcssa
> 
> for.body:                                         ; preds = %for.body, %entry
>   %i.08 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
>   %b.07 = phi i32 [ 0, %entry ], [ %or, %for.body ]
>   %shr = lshr i32 %a, %i.08
>   %and = and i32 %shr, 1
>   %sub = sub nuw nsw i32 31, %i.08
>   %shl = shl i32 %and, %sub
>   %or = or i32 %shl, %b.07
>   %inc = add nuw nsw i32 %i.08, 1
>   %exitcond = icmp eq i32 %inc, 32
>   br i1 %exitcond, label %for.cond.cleanup, label %for.body, !llvm.loop !3
> }
> *** IR Dump After CodeGen Prepare ***
> ; Function Attrs: norecurse nounwind readnone
> define i32 @f(i32 %a) #0 {
> for.body:
>   %rev = call i32 @llvm.bitreverse.i32(i32 %a)
>   ret i32 %rev
> }
> 
> On Tue, Mar 22, 2016 at 3:59 PM, Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
> Hi George,
> 
> Would it be easy to teach CGP how to update it?
> 
> We have the availableIf idiom or something for such cases and that would avoid invalidating the whole thing.
> 
> (Just asking, I don’t actually what in CGP invalidates the DomTree.)
> 
> Cheers,
> -Quentin
> > On Mar 22, 2016, at 2:25 PM, George Burgess IV via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> >
> > Author: gbiv
> > Date: Tue Mar 22 16:25:08 2016
> > New Revision: 264099
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=264099&view=rev <http://llvm.org/viewvc/llvm-project?rev=264099&view=rev>
> > Log:
> > Keep CodeGenPrepare from preserving the domtree.
> >
> > CGP modifies the domtree in some cases, so saying that it preserves the
> > domtree is a lie. We'll be able to selectively preserve it with the new
> > pass manager.
> >
> > Differential Revision: http://reviews.llvm.org/D16893 <http://reviews.llvm.org/D16893>
> >
> > Added:
> >    llvm/trunk/test/Transforms/CodeGenPrepare/dom-tree.ll
> > Modified:
> >    llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
> >
> > Modified: llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=264099&r1=264098&r2=264099&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=264099&r1=264098&r2=264099&view=diff>
> > ==============================================================================
> > --- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Tue Mar 22 16:25:08 2016
> > @@ -158,7 +158,7 @@ class TypePromotionTransaction;
> >     const char *getPassName() const override { return "CodeGen Prepare"; }
> >
> >     void getAnalysisUsage(AnalysisUsage &AU) const override {
> > -      AU.addPreserved<DominatorTreeWrapperPass>();
> > +      // FIXME: When we can selectively preserve passes, preserve the domtree.
> >       AU.addRequired<TargetLibraryInfoWrapperPass>();
> >       AU.addRequired<TargetTransformInfoWrapperPass>();
> >     }
> > @@ -5277,6 +5277,7 @@ bool CodeGenPrepare::optimizeBlock(Basic
> >     for (auto &I : reverse(BB)) {
> >       if (makeBitReverse(I, *DL, *TLI)) {
> >         MadeBitReverse = MadeChange = true;
> > +        ModifiedDT = true;
> >         break;
> >       }
> >     }
> >
> > Added: llvm/trunk/test/Transforms/CodeGenPrepare/dom-tree.ll
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/dom-tree.ll?rev=264099&view=auto <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/dom-tree.ll?rev=264099&view=auto>
> > ==============================================================================
> > --- llvm/trunk/test/Transforms/CodeGenPrepare/dom-tree.ll (added)
> > +++ llvm/trunk/test/Transforms/CodeGenPrepare/dom-tree.ll Tue Mar 22 16:25:08 2016
> > @@ -0,0 +1,41 @@
> > +; RUN: opt -S -loop-unroll -codegenprepare < %s -domtree -analyze | FileCheck %s
> > +;
> > +; Checks that the dom tree is properly invalidated after an operation that will
> > +; invalidate it in CodeGenPrepare. If the domtree isn't properly invalidated,
> > +; this will likely segfault, or print badref.
> > +
> > +; CHECK-NOT: <badref>
> > +
> > +target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
> > +target triple = "armv7--linux-gnueabihf"
> > +
> > +define i32 @f(i32 %a) #0 {
> > +entry:
> > +  br label %for.body
> > +
> > +for.cond.cleanup:
> > +  ret i32 %or
> > +
> > +for.body:
> > +  %i.08 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
> > +  %b.07 = phi i32 [ 0, %entry ], [ %or, %for.body ]
> > +  %shr = lshr i32 %a, %i.08
> > +  %and = and i32 %shr, 1
> > +  %sub = sub nuw nsw i32 31, %i.08
> > +  %shl = shl i32 %and, %sub
> > +  %or = or i32 %shl, %b.07
> > +  %inc = add nuw nsw i32 %i.08, 1
> > +  %exitcond = icmp eq i32 %inc, 32
> > +  br i1 %exitcond, label %for.cond.cleanup, label %for.body, !llvm.loop !3
> > +}
> > +
> > +attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="cortex-a8" "target-features"="+dsp,+neon,+vfp3" "unsafe-fp-math"="false" "use-soft-float"="false" }
> > +
> > +!llvm.module.flags = !{!0, !1}
> > +!llvm.ident = !{!2}
> > +
> > +!0 = !{i32 1, !"wchar_size", i32 4}
> > +!1 = !{i32 1, !"min_enum_size", i32 4}
> > +!2 = !{!"clang version 3.8.0 (http://llvm.org/git/clang.git <http://llvm.org/git/clang.git> b7441a0f42c43a8eea9e3e706be187252db747fa)"}
> > +!3 = distinct !{!3, !4}
> > +!4 = !{!"llvm.loop.unroll.full"}
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://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/20160322/9ee0ab22/attachment.html>


More information about the llvm-commits mailing list