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

Tue Nov 11 16:35:40 PST 2014

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.

-- 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141111/738d5f44/attachment.html>