Index: lib/Transforms/IPO/FunctionAttrs.cpp =================================================================== --- lib/Transforms/IPO/FunctionAttrs.cpp +++ lib/Transforms/IPO/FunctionAttrs.cpp @@ -531,6 +531,43 @@ return Changed; } +/// 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 attribute +/// knowledge may disappear with the call. +static bool addArgumentAttrsFromCallsites(Function &F) { + bool PropagatedAttr = false; + + // 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. + BasicBlock &Entry = F.getEntryBlock(); + for (Instruction &I : Entry) { + if (auto *Call = dyn_cast(&I)) { + unsigned ArgIndex = 0; + for (Value *Arg : Call->arg_operands()) { + ArgIndex++; + auto *CallerArg = dyn_cast(Arg); + // If any argument to 'F' (the caller) is known non-null at the callsite + // and that call is guaranteed to execute, then the argument must be + // non-null throughout 'F'. + if (CallerArg && !CallerArg->hasNonNullAttr() && + Call->paramHasAttr(ArgIndex, Attribute::NonNull)) { + CallerArg->addAttr(Attribute::NonNull); + PropagatedAttr = true; + } + } + } + // Verify that all instructions before a call will execute sequentially. + if (!isGuaranteedToTransferExecutionToSuccessor(&I)) + break; + } + + return PropagatedAttr; +} + /// Deduce nocapture attributes for the SCC. static bool addArgumentAttrs(const SCCNodeSet &SCCNodes) { bool Changed = false; @@ -549,6 +586,8 @@ if (!F->hasExactDefinition()) continue; + Changed |= addArgumentAttrsFromCallsites(*F); + // Functions that are readonly (or readnone) and nounwind and don't return // a value can't capture arguments. Don't analyze them. if (F->onlyReadsMemory() && F->doesNotThrow() && Index: test/Transforms/FunctionAttrs/nonnull.ll =================================================================== --- test/Transforms/FunctionAttrs/nonnull.ll +++ test/Transforms/FunctionAttrs/nonnull.ll @@ -71,4 +71,117 @@ ret i8* %phi } +; 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); + +declare i8 @use1readonly(i8* %x) readonly ; readonly guarantees that execution continues to successor + +; Can't extend non-null to parent for any argument because the 2nd call is not guaranteed to execute. + +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. + +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* %c, i8* %a, i8* %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. + +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* %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. + +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* %c) +; CHECK-NEXT: call void @use1(i8* %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 +; 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 +} + +; The nonnull callsite is guaranteed to execute, so the argument must be nonnull throughout the parent. + +define i8 @parent7(i8* %a) { +; CHECK-LABEL: @parent7(i8* nonnull %a) +; CHECK-NEXT: [[RET:%.*]] = call i8 @use1readonly(i8* %a) +; CHECK-NEXT: call void @use1nonnull(i8* %a) +; CHECK-NEXT: ret i8 [[RET]] +; + %ret = call i8 @use1readonly(i8* %a) + call void @use1nonnull(i8* %a) + ret i8 %ret +}