[llvm] r221737 - Canonicalize an assume(load != null) into !nonnull metadata

Tue Nov 11 17:05:34 PST 2014

Thanks for the responses everyone.

On Tue, Nov 11, 2014 at 4:45 PM, Hal Finkel <hfinkel at anl.gov> wrote:

> ----- Original Message -----
> > From: "Sean Silva" <chisophugis at gmail.com>
> > To: "Hal Finkel" <hfinkel at anl.gov>
> > Cc: "David Blaikie" <dblaikie at gmail.com>, llvm-commits at cs.uiuc.edu
> > Sent: Tuesday, November 11, 2014 6:35:40 PM
> > Subject: Re: [llvm] r221737 - Canonicalize an assume(load != null) into
> !nonnull metadata
> >
> > On Tue, Nov 11, 2014 at 4:15 PM, Hal Finkel < hfinkel at anl.gov >
> > wrote:
> >
> > ----- Original Message -----
> > > From: "David Blaikie" < dblaikie at gmail.com >
> > > To: "Philip Reames" < listmail at philipreames.com >
> > > Cc: llvm-commits at cs.uiuc.edu
> > > Sent: Tuesday, November 11, 2014 5:47:28 PM
> > > Subject: Re: [llvm] r221737 - Canonicalize an assume(load != null)
> > > into !nonnull metadata
> > >
> > > On Tue, Nov 11, 2014 at 3:33 PM, Philip Reames <
> > > listmail at philipreames.com > wrote:
> > >
> > >
> > > Author: reames
> > > Date: Tue Nov 11 17:33:19 2014
> > > New Revision: 221737
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=221737&view=rev
> > > Log:
> > > Canonicalize an assume(load != null) into !nonnull metadata
> > >
> > > We currently have two ways of informing the optimizer that the
> > > result
> > > of a load is never null: metadata and assume. This change converts
> > > the second in to the former. This avoids a need to implement
> > > optimizations using both forms.
> > >
> > > We should probably extend this basic idea to metadata of other
> > > forms;
> > > in particular, range metadata. We view is that assumes should be
> > > considered a "last resort" for when there isn't a more canonical
> > > way
> > > to represent something.
> > >
> > >
> > > (naively) seems almost a pity, really - if we have the general
> > > tool,
> > > why not drop the special cases & just use the general one? For
> > > things already using the special cases, they can continue to look
> > > for the very specific canonicalized case of the general form with
> > > relatively low cost, I would imagine - but I know very little about
> > > all of this. Just a thought.
> > >
> >
> > I share your desire for making use of a single general-purpose tool,
> > but in this case, using @llvm.assume as a much higher cost compared
> > to the metadata (both in its representational size and in its
> > ability to block potentially-useful optimizations),
> >
> >
> > How does it block optimizations? That's sort of saddening.
>
> There are essentially three ways:
>
>  1. By adding uses on the values on which assumptions are being placed
> (hasOneUse no longer returns true).
>
>  2. By blocking code motion (I've tried to adjust for this by placing
> special knowledge inside of BasicAA, EarlyCSE, LoopVectorize, etc. -- but I
> can't say I've optimally gotten everything).
>
>  3. By keeping otherwise dead blocks alive (we could have a side of a
> control-flow diamond with nothing but an assumption) -- hopefully the
> MI-level passes will clean up properly if this happens (and they normally
> do), but it could still block otherwise useful IR-level optimizations.
>
> The general philosophy behind using @llvm.assume is: Use it only when
> you're fairly certain that the information you're providing is so important
> that it is worth preserving that information accounting for the fact that
> the preservation has priority over optimization.
>
>  -Hal
>
> >
> >
> > -- Sean Silva
> >
> >
> >
> >
> > so I think it makes sense to canonicalize on the metadata when
> > available.
> >
> > -Hal
> >
> >
> >
> > >
> > >
> > > Reviewed by: Hal
> > > Differential Revision: http://reviews.llvm.org/D5951
> > >
> > >
> > > Modified:
> > > llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
> > > llvm/trunk/test/Transforms/InstCombine/assume.ll
> > >
> > > Modified:
> > > llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
> > > URL:
> > >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=221737&r1=221736&r2=221737&view=diff
> > >
> ==============================================================================
> > > --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
> > > (original)
> > > +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Tue
> > > Nov 11 17:33:19 2014
> > > @@ -1102,6 +1102,26 @@ Instruction *InstCombiner::visitCallInst
> > > return EraseInstFromFunction(*II);
> > > }
> > >
> > > + // assume( (load addr) != null ) -> add 'nonnull' metadata to
> > > load
> > > + // (if assume is valid at the load)
> > > + if (ICmpInst* ICmp = dyn_cast<ICmpInst>(IIOperand)) {
> > > + Value *LHS = ICmp->getOperand(0);
> > > + Value *RHS = ICmp->getOperand(1);
> > > + if (ICmpInst::ICMP_NE == ICmp->getPredicate() &&
> > > + isa<LoadInst>(LHS) &&
> > > + isa<Constant>(RHS) &&
> > > + RHS->getType()->isPointerTy() &&
> > > + cast<Constant>(RHS)->isNullValue()) {
> > > + LoadInst* LI = cast<LoadInst>(LHS);
> > > + if (isValidAssumeForContext(II, LI, DL, DT)) {
> > > + MDNode* MD = MDNode::get(II->getContext(), ArrayRef<Value*>());
> > > + LI->setMetadata(LLVMContext::MD_nonnull, MD);
> > > + return EraseInstFromFunction(*II);
> > > + }
> > > + }
> > > + // TODO: apply nonnull return attributes to calls and invokes
> > > + // TODO: apply range metadata for range check patterns?
> > > + }
> > > // If there is a dominating assume with the same condition as this
> > > one,
> > > // then this one is redundant, and should be removed.
> > > APInt KnownZero(1, 0), KnownOne(1, 0);
> > >
> > > Modified: llvm/trunk/test/Transforms/InstCombine/assume.ll
> > > URL:
> > >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/assume.ll?rev=221737&r1=221736&r2=221737&view=diff
> > >
> ==============================================================================
> > > --- llvm/trunk/test/Transforms/InstCombine/assume.ll (original)
> > > +++ llvm/trunk/test/Transforms/InstCombine/assume.ll Tue Nov 11
> > > 17:33:19 2014
> > > @@ -186,6 +186,80 @@ entry:
> > > ; CHECK: ret i32 0
> > > }
> > >
> > > +declare void @escape(i32* %a)
> > > +
> > > +; Do we canonicalize a nonnull assumption on a load into
> > > +; metadata form?
> > > +define i1 @nonnull1(i32** %a) {
> > > +entry:
> > > + %load = load i32** %a
> > > + %cmp = icmp ne i32* %load, null
> > > + tail call void @llvm.assume(i1 %cmp)
> > > + tail call void @escape(i32* %load)
> > > + %rval = icmp eq i32* %load, null
> > > + ret i1 %rval
> > > +
> > > +; CHECK-LABEL: @nonnull1
> > > +; CHECK: !nonnull
> > > +; CHECK-NOT: call void @llvm.assume
> > > +; CHECK: ret i1 false
> > > +}
> > > +
> > > +; Make sure the above canonicalization applies only
> > > +; to pointer types. Doing otherwise would be illegal.
> > > +define i1 @nonnull2(i32* %a) {
> > > +entry:
> > > + %load = load i32* %a
> > > + %cmp = icmp ne i32 %load, 0
> > > + tail call void @llvm.assume(i1 %cmp)
> > > + %rval = icmp eq i32 %load, 0
> > > + ret i1 %rval
> > > +
> > > +; CHECK-LABEL: @nonnull2
> > > +; CHECK-NOT: !nonnull
> > > +; CHECK: call void @llvm.assume
> > > +}
> > > +
> > > +; Make sure the above canonicalization does not trigger
> > > +; if the assume is control dependent on something else
> > > +define i1 @nonnull3(i32** %a, i1 %control) {
> > > +entry:
> > > + %load = load i32** %a
> > > + %cmp = icmp ne i32* %load, null
> > > + br i1 %control, label %taken, label %not_taken
> > > +taken:
> > > + tail call void @llvm.assume(i1 %cmp)
> > > + %rval = icmp eq i32* %load, null
> > > + ret i1 %rval
> > > +not_taken:
> > > + ret i1 true
> > > +
> > > +; CHECK-LABEL: @nonnull3
> > > +; CHECK-NOT: !nonnull
> > > +; CHECK: call void @llvm.assume
> > > +}
> > > +
> > > +; Make sure the above canonicalization does not trigger
> > > +; if the path from the load to the assume is potentially
> > > +; interrupted by an exception being thrown
> > > +define i1 @nonnull4(i32** %a) {
> > > +entry:
> > > + %load = load i32** %a
> > > + ;; This call may throw!
> > > + tail call void @escape(i32* %load)
> > > + %cmp = icmp ne i32* %load, null
> > > + tail call void @llvm.assume(i1 %cmp)
> > > + %rval = icmp eq i32* %load, null
> > > + ret i1 %rval
> > > +
> > > +; CHECK-LABEL: @nonnull4
> > > +; CHECK-NOT: !nonnull
> > > +; CHECK: call void @llvm.assume
> > > +}
> > > +
> > > +
> > > +
> > > +
> > > attributes #0 = { nounwind uwtable }
> > > attributes #1 = { nounwind }
> > >
> > >
> > >
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >
> > >
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >
> >
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141111/b7da21d6/attachment.html>