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

Tue Nov 11 16:45:41 PST 2014

----- 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