diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp --- a/llvm/lib/Transforms/Utils/InlineFunction.cpp +++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp @@ -89,6 +89,10 @@ "update-return-attrs", cl::init(true), cl::Hidden, cl::desc("Update return attributes on calls within inlined body")); +static cl::opt UpdateLoadMetadataDuringInlining( + "update-load-metadata-during-inlining", cl::init(true), cl::Hidden, + cl::desc("Update metadata on loads within inlined body")); + static cl::opt InlinerAttributeWindow( "max-inst-checked-for-throw-during-inlining", cl::Hidden, cl::desc("the maximum number of instructions analyzed for may throw during " @@ -1182,7 +1186,7 @@ } static void AddReturnAttributes(CallSite CS, ValueToValueMapTy &VMap) { - if (!UpdateReturnAttributes) + if (!UpdateReturnAttributes && !UpdateLoadMetadataDuringInlining) return; AttrBuilder Valid = IdentifyValidAttributes(CS); @@ -1191,18 +1195,32 @@ auto *CalledFunction = CS.getCalledFunction(); auto &Context = CalledFunction->getContext(); + auto getExpectedRV = [&](Value *V) -> Instruction * { + if (UpdateReturnAttributes && isa(V)) + return dyn_cast_or_null(VMap.lookup(V)); + if (UpdateLoadMetadataDuringInlining && isa(V)) + return dyn_cast_or_null(VMap.lookup(V)); + return nullptr; + }; + + MDBuilder MDB(Context); + auto CreateMDNode = [&](uint64_t Num) -> MDNode * { + auto *Int = ConstantInt::get(Type::getInt64Ty(Context), Num); + return MDNode::get(Context, MDB.createConstant(Int)); + }; + for (auto &BB : *CalledFunction) { auto *RI = dyn_cast(BB.getTerminator()); - if (!RI || !isa(RI->getOperand(0))) + if (!RI) continue; - auto *RetVal = cast(RI->getOperand(0)); // Sanity check that the cloned RetVal exists and is a call, otherwise we // cannot add the attributes on the cloned RetVal. // Simplification during inlining could have transformed the cloned // instruction. - auto *NewRetVal = dyn_cast_or_null(VMap.lookup(RetVal)); + auto *NewRetVal = getExpectedRV(RI->getOperand(0)); if (!NewRetVal) continue; + auto *RetVal = cast(RI->getOperand(0)); // Backward propagation of attributes to the returned value may be incorrect // if it is control flow dependent. // Consider: @@ -1230,10 +1248,27 @@ // with a differing value, the AttributeList's merge API honours the already // existing attribute value (i.e. attributes such as dereferenceable, // dereferenceable_or_null etc). See AttrBuilder::merge for more details. - AttributeList AL = NewRetVal->getAttributes(); - AttributeList NewAL = - AL.addAttributes(Context, AttributeList::ReturnIndex, Valid); - NewRetVal->setAttributes(NewAL); + if (auto *CB = dyn_cast(NewRetVal)) { + AttributeList AL = CB->getAttributes(); + AttributeList NewAL = + AL.addAttributes(Context, AttributeList::ReturnIndex, Valid); + CB->setAttributes(NewAL); + } else { + auto *NewLI = cast(NewRetVal); + if (CS.isReturnNonNull()) + NewLI->setMetadata(LLVMContext::MD_nonnull, CreateMDNode(1)); + // If the load already has a dereferenceable/dereferenceable_or_null + // metadata, we should honour it. + if (uint64_t DerefBytes = Valid.getDereferenceableBytes()) + if(!NewLI->getMetadata(LLVMContext::MD_dereferenceable)) + NewLI->setMetadata(LLVMContext::MD_dereferenceable, + CreateMDNode(DerefBytes)); + if (uint64_t DerefOrNullBytes = Valid.getDereferenceableOrNullBytes()) + if (!NewLI->getMetadata(LLVMContext::MD_dereferenceable_or_null)) + NewLI->setMetadata(LLVMContext::MD_dereferenceable_or_null, + CreateMDNode(DerefOrNullBytes)); + } + } } diff --git a/llvm/test/Transforms/Inline/ret_load_metadata.ll b/llvm/test/Transforms/Inline/ret_load_metadata.ll new file mode 100644 --- /dev/null +++ b/llvm/test/Transforms/Inline/ret_load_metadata.ll @@ -0,0 +1,103 @@ +; RUN: opt < %s -inline-threshold=0 -update-load-metadata-during-inlining=true -always-inline -S | FileCheck %s +; RUN: opt < %s -passes=always-inline -update-load-metadata-during-inlining=true -S | FileCheck %s + + +define i8* @callee(i8** %p) alwaysinline { +; CHECK: @callee( +; CHECK-NOT: nonnull + %r = load i8*, i8** %p, align 8 + ret i8* %r +} + +define i8* @test(i8** %ptr, i64 %x) { +; CHECK-LABEL: @test +; CHECK: load i8*, i8** %gep, align 8, !nonnull !0 + %gep = getelementptr inbounds i8*, i8** %ptr, i64 %x + %p = call nonnull i8* @callee(i8** %gep) + ret i8* %p +} + +declare void @does_not_return(i8*) nounwind +define internal i8* @callee_negative(i8** %p) alwaysinline { +; CHECK-NOT: @callee_negative( + %r = load i8*, i8** %p, align 8 + call void @does_not_return(i8* %r) + ret i8* %r +} + +define i8* @negative_test(i8** %ptr, i64 %x) { +; CHECK-LABEL: @negative_test +; CHECK: load i8*, i8** %gep, align 8 +; CHECK-NOT: nonnull + %gep = getelementptr inbounds i8*, i8** %ptr, i64 %x + %p = call nonnull i8* @callee_negative(i8** %gep) + ret i8* %p +} + + +define internal i8* @callee2(i8** %p) alwaysinline { +; CHECK-NOT: @callee2( + %r = load i8*, i8** %p, align 8 + ret i8* %r +} + +; dereferenceable attribute in default addrspace implies nonnull +define i8* @test2(i8** %ptr, i64 %x) { +; CHECK-LABEL: @test2 +; CHECK: load i8*, i8** %gep, align 8, !nonnull !0, !dereferenceable !1 + %gep = getelementptr inbounds i8*, i8** %ptr, i64 %x + %p = call dereferenceable(12) i8* @callee(i8** %gep) + ret i8* %p +} + +declare void @bar(i8 addrspace(1)*) argmemonly nounwind + +define internal i8 addrspace(1)* @callee3(i8 addrspace(1)* addrspace(1)* %p) alwaysinline { + %r = load i8 addrspace(1)*, i8 addrspace(1)* addrspace(1)* %p, align 8 + call void @bar(i8 addrspace(1)* %r) + ret i8 addrspace(1)* %r +} + +; This load in callee already has a dereferenceable_or_null metadata. We should +; honour it. +define internal i8 addrspace(1)* @callee5(i8 addrspace(1)* addrspace(1)* %p) alwaysinline { + %r = load i8 addrspace(1)*, i8 addrspace(1)* addrspace(1)* %p, align 8, !dereferenceable_or_null !3 + call void @bar(i8 addrspace(1)* %r) + ret i8 addrspace(1)* %r +} + +define i8 addrspace(1)* @test3(i8 addrspace(1)* addrspace(1)* %ptr, i64 %x) { +; CHECK-LABEL: @test3 +; CHECK: load i8 addrspace(1)*, i8 addrspace(1)* addrspace(1)* %gep, align 8, !dereferenceable_or_null !2 +; CHECK: load i8 addrspace(1)*, i8 addrspace(1)* addrspace(1)* %ptr, align 8, !dereferenceable_or_null !3 + %gep = getelementptr inbounds i8 addrspace(1)*, i8 addrspace(1)* addrspace(1)* %ptr, i64 %x + %p = call dereferenceable_or_null(16) i8 addrspace(1)* @callee3(i8 addrspace(1)* addrspace(1)* %gep) + %q = call dereferenceable_or_null(20) i8 addrspace(1)* @callee5(i8 addrspace(1)* addrspace(1)* %ptr) + ret i8 addrspace(1)* %p +} + +; attribute is part of the callee itself +define nonnull i8* @callee4(i8** %p) alwaysinline { + %r = load i8*, i8** %p, align 8 + ret i8* %r +} + +; TODO : We should infer the attribute on the callee +; and add the nonnull on the load +define i8* @test4(i8** %ptr, i64 %x) { +; CHECK-LABEL: @test4 +; CHECK: load i8*, i8** %gep, align 8 +; CHECK-NOT: nonnull + %gep = getelementptr inbounds i8*, i8** %ptr, i64 %x + %p = call i8* @callee(i8** %gep) + ret i8* %p +} + +!0 = !{i64 1} +!1 = !{i64 12} +!2 = !{i64 16} +!3 = !{i64 24} +; CHECK: !0 = !{i64 1} +; CHECK: !1 = !{i64 12} +; CHECK: !2 = !{i64 16} +; CHECK: !3 = !{i64 24}