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

[Hal Finkel]

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