[llvm] 1d0f757 - [InlineFunction] Update metadata on loads that are return values
Anna Thomas via llvm-commits
llvm-commits at lists.llvm.org
Sun Apr 5 11:51:44 PDT 2020
Author: Anna Thomas
Date: 2020-04-05T14:50:10-04:00
New Revision: 1d0f757904919d19f1cf5dcd307874bceb1e9efb
URL: https://github.com/llvm/llvm-project/commit/1d0f757904919d19f1cf5dcd307874bceb1e9efb
DIFF: https://github.com/llvm/llvm-project/commit/1d0f757904919d19f1cf5dcd307874bceb1e9efb.diff
LOG: [InlineFunction] Update metadata on loads that are return values
This patch builds upon D76140 by updating metadata on pointer typed
loads in inlined functions, when the load is the return value, and the
callsite contains return attributes which can be updated as metadata on
the load.
Added test cases show this for nonnull, dereferenceable,
dereferenceable_or_null
Reviewed-By: jdoerfert
Differential Revision: https://reviews.llvm.org/D76792
Added:
llvm/test/Transforms/Inline/ret_load_metadata.ll
Modified:
llvm/lib/Transforms/Utils/InlineFunction.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 5fe651f61155..2c86af68d646 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -89,6 +89,10 @@ static cl::opt<bool> UpdateReturnAttributes(
"update-return-attrs", cl::init(true), cl::Hidden,
cl::desc("Update return attributes on calls within inlined body"));
+static cl::opt<bool> UpdateLoadMetadataDuringInlining(
+ "update-load-metadata-during-inlining", cl::init(true), cl::Hidden,
+ cl::desc("Update metadata on loads within inlined body"));
+
static cl::opt<unsigned> 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 AttrBuilder IdentifyValidAttributes(CallSite CS) {
}
static void AddReturnAttributes(CallSite CS, ValueToValueMapTy &VMap) {
- if (!UpdateReturnAttributes)
+ if (!UpdateReturnAttributes && !UpdateLoadMetadataDuringInlining)
return;
AttrBuilder Valid = IdentifyValidAttributes(CS);
@@ -1191,18 +1195,32 @@ static void AddReturnAttributes(CallSite CS, ValueToValueMapTy &VMap) {
auto *CalledFunction = CS.getCalledFunction();
auto &Context = CalledFunction->getContext();
+ auto getExpectedRV = [&](Value *V) -> Instruction * {
+ if (UpdateReturnAttributes && isa<CallBase>(V))
+ return dyn_cast_or_null<CallBase>(VMap.lookup(V));
+ if (UpdateLoadMetadataDuringInlining && isa<LoadInst>(V))
+ return dyn_cast_or_null<LoadInst>(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<ReturnInst>(BB.getTerminator());
- if (!RI || !isa<CallBase>(RI->getOperand(0)))
+ if (!RI)
continue;
- auto *RetVal = cast<CallBase>(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<CallBase>(VMap.lookup(RetVal));
+ auto *NewRetVal = getExpectedRV(RI->getOperand(0));
if (!NewRetVal)
continue;
+ auto *RetVal = cast<Instruction>(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 @@ static void AddReturnAttributes(CallSite CS, ValueToValueMapTy &VMap) {
// with a
diff ering 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<CallBase>(NewRetVal)) {
+ AttributeList AL = CB->getAttributes();
+ AttributeList NewAL =
+ AL.addAttributes(Context, AttributeList::ReturnIndex, Valid);
+ CB->setAttributes(NewAL);
+ } else {
+ auto *NewLI = cast<LoadInst>(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
index 000000000000..13dabb074017
--- /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}
More information about the llvm-commits
mailing list