Index: lib/Transforms/InstCombine/InstCombineCalls.cpp =================================================================== --- lib/Transforms/InstCombine/InstCombineCalls.cpp +++ lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -2776,6 +2776,66 @@ return nullptr; } +/// If a callsite has arguments that are also arguments to the parent function, +/// try to propagate attributes from the callsite's arguments to the parent's +/// arguments. In general, we don't alter the IR to preserve analysis, but in +/// this case inlining may cause information loss because the nonnull knowledge +/// may disappear with the call. +static Instruction *extendArgAttrsToCaller(CallSite &CS, DominatorTree &DT) { + // Make sure that this callsite is part of a function. + BasicBlock *BB = CS.getParent(); + if (!BB) + return nullptr; + Function *F = BB->getParent(); + if (!F) + return nullptr; + + // For an argument attribute to transfer from a callsite to the parent, the + // call must be guaranteed to execute every time the parent is called. + // Conservatively, just check if the callsite is in the entry block. This + // could be relaxed to test if the callsite post-dominates the entry block. + if (&F->getEntryBlock() != BB) + return nullptr; + + // Verify that all instructions before the call will execute sequentially. + for (Instruction &I : F->getEntryBlock()) { + if (&I == CS.getInstruction()) + break; + if (!isGuaranteedToTransferExecutionToSuccessor(&I)) + return nullptr; + } + + // Collect parameters that are known non-null at this callsite. + SmallVector NonNullParams; + unsigned ArgIndex = 0; + for (Value *V : CS.args()) + if (CS.paramHasAttr(++ArgIndex, Attribute::NonNull)) + NonNullParams.push_back(V); + + Instruction *Call = CS.getInstruction(); + auto CallDominatesOtherUsesOfValue = [&Call, &DT](Value *V) { + for (User *U : V->users()) + if (U != Call && !DT.dominates(Call, cast(U))) + return false; + return true; + }; + + // If any argument to the caller is known non-null at the callsite and the + // call dominates all other uses of that argument, then the argument is + // non-null throughout the caller. + bool PropagatedNonNull = false; + for (Value *V : NonNullParams) { + auto *CallerArg = dyn_cast(V); + if (CallerArg && !CallerArg->hasNonNullAttr() && + CallDominatesOtherUsesOfValue(V)) { + CallerArg->addAttr(Attribute::NonNull); + PropagatedNonNull = true; + } + } + + return PropagatedNonNull ? Call : nullptr; +} + /// Improvements for call and invoke instructions. Instruction *InstCombiner::visitCallSite(CallSite CS) { if (isAllocLikeFn(CS.getInstruction(), &TLI)) @@ -2784,8 +2844,10 @@ 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. + // attribute. This is helpful for inlining calls to functions with null + // checks on their arguments. In general, we don't alter the IR to preserve + // analysis, but in this case inlining may cause information loss because + // the nonnull knowledge may disappear with the call. SmallVector Indices; unsigned ArgNo = 0; @@ -2808,6 +2870,9 @@ Changed = true; } + if (Instruction *I = extendArgAttrsToCaller(CS, DT)) + return I; + // If the callee is a pointer to a function, attempt to move any casts to the // arguments of the call/invoke. Value *Callee = CS.getCalledValue(); Index: test/Transforms/InstCombine/call_nonnull_arg.ll =================================================================== --- test/Transforms/InstCombine/call_nonnull_arg.ll +++ test/Transforms/InstCombine/call_nonnull_arg.ll @@ -1,6 +1,8 @@ -; NOTE: Assertions have been autogenerated by utils/update_test_checks.py ; RUN: opt < %s -instcombine -S | FileCheck %s +; Assertions are enhanced from the auto-generated checks in utils/update_test_checks.py +; to include parameters on some of the check label lines. + ; InstCombine should mark null-checked argument as nonnull at callsite declare void @dummy(i32*, i32) @@ -31,3 +33,102 @@ unreachable } +; Test propagation of nonnull callsite args back to caller. + +declare void @use1(i8* %x); +declare void @use2(i8* %x, i8* %y); +declare void @use3(i8* %x, i8* %y, i8* %z); + +declare void @use1nonnull(i8* nonnull %x); +declare void @use2nonnull(i8* nonnull %x, i8* nonnull %y); +declare void @use3nonnull(i8* nonnull %x, i8* nonnull %y, i8* nonnull %z); + +; Can't extend non-null to parent for any argument. + +define void @parent1(i8* %a, i8* %b, i8* %c) { +; CHECK-LABEL: @parent1(i8* %a, i8* %b, i8* %c) +; CHECK-NEXT: call void @use3(i8* %c, i8* %a, i8* %b) +; CHECK-NEXT: call void @use3nonnull(i8* %b, i8* %c, i8* %a) +; CHECK-NEXT: ret void +; + call void @use3(i8* %c, i8* %a, i8* %b) + call void @use3nonnull(i8* %b, i8* %c, i8* %a) + ret void +} + +; Extend non-null to parent for all arguments. Non-null then extends to the 2nd call from the parent. + +define void @parent2(i8* %a, i8* %b, i8* %c) { +; CHECK-LABEL: @parent2(i8* nonnull %a, i8* nonnull %b, i8* nonnull %c) +; CHECK-NEXT: call void @use3nonnull(i8* %b, i8* %c, i8* %a) +; CHECK-NEXT: call void @use3(i8* nonnull %c, i8* nonnull %a, i8* nonnull %b) +; CHECK-NEXT: ret void +; + call void @use3nonnull(i8* %b, i8* %c, i8* %a) + call void @use3(i8* %c, i8* %a, i8* %b) + ret void +} + +; Extend non-null to parent for 1st argument. Non-null then extends to the 2nd call from the parent. + +define void @parent3(i8* %a, i8* %b, i8* %c) { +; CHECK-LABEL: @parent3(i8* nonnull %a, i8* %b, i8* %c) +; CHECK-NEXT: call void @use1nonnull(i8* %a) +; CHECK-NEXT: call void @use3(i8* %c, i8* %b, i8* nonnull %a) +; CHECK-NEXT: ret void +; + call void @use1nonnull(i8* %a) + call void @use3(i8* %c, i8* %b, i8* %a) + ret void +} + +; Extend non-null to parent for last 2 arguments. Non-null then extends to the 2nd and 3rd calls from the parent. + +define void @parent4(i8* %a, i8* %b, i8* %c) { +; CHECK-LABEL: @parent4(i8* %a, i8* nonnull %b, i8* nonnull %c) +; CHECK-NEXT: call void @use2nonnull(i8* %c, i8* %b) +; CHECK-NEXT: call void @use2(i8* %a, i8* nonnull %c) +; CHECK-NEXT: call void @use1(i8* nonnull %b) +; CHECK-NEXT: ret void +; + call void @use2nonnull(i8* %c, i8* %b) + call void @use2(i8* %a, i8* %c) + call void @use1(i8* %b) + ret void +} + +; The callsite must execute in order for the attribute to transfer to the parent. +; It appears benign to extend non-null to the parent in this case, but we can't do that +; because it would incorrectly propagate the wrong information to its callers. + +define void @parent5(i8* %a, i1 %a_is_notnull) { +; CHECK-LABEL: @parent5(i8* %a, i1 %a_is_notnull) +; CHECK-NEXT: br i1 %a_is_notnull, label %t, label %f +; CHECK: t: +; CHECK-NEXT: call void @use1nonnull(i8* %a) +; CHECK-NEXT: ret void +; CHECK: f: +; CHECK-NEXT: ret void +; + br i1 %a_is_notnull, label %t, label %f +t: + call void @use1nonnull(i8* %a) + ret void +f: + ret void +} + +; The callsite must execute in order for the attribute to transfer to the parent. +; The volatile load might trap, so there's no guarantee that we'll ever get to the call. + +define i8 @parent6(i8* %a, i8* %b) { +; CHECK-LABEL: @parent6(i8* %a, i8* %b) +; CHECK-NEXT: [[C:%.*]] = load volatile i8, i8* %b, align 1 +; CHECK-NEXT: call void @use1nonnull(i8* %a) +; CHECK-NEXT: ret i8 [[C]] +; + %c = load volatile i8, i8* %b + call void @use1nonnull(i8* %a) + ret i8 %c +} +