[InstCombineCalls] Use isKnownNonNullAt() to check nullness of passing arguments at callsite

[• chenli]

Summary

This patch replaces isKnownNonNull() with isKnownNonNullAt() when checking nullness of passing arguments at callsite. In this way it can handle cases where the argument does not have nonnull attribute but has a dominating null check from the CFG. It also adds assertions in isKnownNonNull() and isKnownNonNullFromDominatingCondition() to make sure the value checked is pointer type (as defined in LLVM document). These assertions might trip failures in things which are not covered under llvm/test, but fixes should be pretty obvious.

[reames]

LGTM w/one minor comment optional.

test/Transforms/InstCombine/call_nonnull_arg.ll

4

Not sure this needs to be it's own file. It could probably just be added to the existing file which contains the previous nonnull attribute tests.

[• chenli]

test/Transforms/InstCombine/call_nonnull_arg.ll

4

I dont think there is an existing test case to check nonnull attribute at callsite. I didn't find one under InstCombine/ directory.

[• chenli]

This update fixed a issue when argument is not pointer type and should not be attached with nonnull attribute.

[reames]

Revised version w/fix LGTM provided test case added.

I'm a bit concerned that we'll see the assert you added trip in something which doesn't have a unit test, but if we do, the fix should be pretty obvious at the call site. Please call this out specifically in the commit message.

[• chenli]

Update test case and summary.

lib/Analysis/ValueTracking.cpp

 }
 
 bool llvm::mayBeMemoryDependent(const Instruction &I) {
   return I.mayReadOrWriteMemory() || !isSafeToSpeculativelyExecute(&I);
 }
 
 /// Return true if we know that the specified value is never null.
 bool llvm::isKnownNonNull(const Value *V, const TargetLibraryInfo *TLI) {
+  assert(V->getType()->isPointerTy() && "V must be pointer type");
+
   // Alloca never returns null, malloc might.
   if (isa<AllocaInst>(V)) return true;
 
   // A byval, inalloca, or nonnull argument is never null.
   if (const Argument *A = dyn_cast<Argument>(V))
     return A->hasByValOrInAllocaAttr() || A->hasNonNullAttr();
 
   // A global variable in address space 0 is non null unless extern weak.
if (isOperatorNewLikeFn(V, TLI, /*LookThroughBitCast=*/true))
     return true;
 
   return false;
 }
 
 static bool isKnownNonNullFromDominatingCondition(const Value *V,
                                                   const Instruction *CtxI,
                                                   const DominatorTree *DT) {
+  assert(V->getType()->isPointerTy() && "V must be pointer type");
+
   unsigned NumUsesExplored = 0;
   for (auto U : V->users()) {
     // Avoid massive lists
     if (NumUsesExplored >= DomConditionsMaxUses)
       break;
     NumUsesExplored++;
     // Consider only compare instructions uniquely controlling a branch
     const ICmpInst *Cmp = dyn_cast<ICmpInst>(U);

lib/Transforms/InstCombine/InstCombineCalls.cpp

Instruction *InstCombiner::visitCallSite(CallSite CS) {
 
   bool Changed = false;
 
   // Mark any parameters that are known to be non-null with the nonnull
   // attribute.  This is helpful for inlining calls to functions with null
   // checks on their arguments.
   unsigned ArgNo = 0;
   for (Value *V : CS.args()) {
-    if (!CS.paramHasAttr(ArgNo+1, Attribute::NonNull) &&
-        isKnownNonNull(V)) {
+    if (V->getType()->isPointerTy() && !CS.paramHasAttr(ArgNo+1, Attribute::NonNull) &&
+        isKnownNonNullAt(V, CS.getInstruction(), DT, TLI)) {
       AttributeSet AS = CS.getAttributes();
       AS = AS.addAttribute(CS.getInstruction()->getContext(), ArgNo+1,
                            Attribute::NonNull);
       CS.setAttributes(AS);
       Changed = true;
     }
     ArgNo++;
   }

test/Transforms/InstCombine/call_nonnull_arg.ll

+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+; InstCombine should mark null-checked argument as nonnull at callsite
+declare void @dummy(i32*, i32)
+
+define void @test(i32* %a, i32 %b) {
+; CHECK-LABEL: @test
+; CHECK: call void @dummy(i32* nonnull %a, i32 %b)
+entry:
+  %cond1 = icmp eq i32* %a, null
+  br i1 %cond1, label %dead, label %not_null
+not_null:
+  %cond2 = icmp eq i32 %b, 0
+  br i1 %cond2, label %dead, label %not_zero
+not_zero:
+  call void @dummy(i32* %a, i32 %b)
+  ret void
+dead:
+  unreachable
+}