[llvm] ef49b1d - Revert "[InlineFunction] Update metadata on loads that are return values"
Anna Thomas via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 17 14:23:17 PDT 2020
Author: Anna Thomas
Date: 2020-04-17T17:23:00-04:00
New Revision: ef49b1d97e1ac75bff8ff7dec3097b43bcd07e73
URL: https://github.com/llvm/llvm-project/commit/ef49b1d97e1ac75bff8ff7dec3097b43bcd07e73
DIFF: https://github.com/llvm/llvm-project/commit/ef49b1d97e1ac75bff8ff7dec3097b43bcd07e73.diff
LOG: Revert "[InlineFunction] Update metadata on loads that are return values"
This reverts commit 1d0f757904919d19f1cf5dcd307874bceb1e9efb because of
https://bugs.llvm.org/show_bug.cgi?id=45590. Needs investigation.
Added:
Modified:
llvm/lib/Transforms/Utils/InlineFunction.cpp
Removed:
llvm/test/Transforms/Inline/ret_load_metadata.ll
################################################################################
diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 9ad9f7b8e11d..92ab8f9b7718 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -88,10 +88,6 @@ 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 "
@@ -1173,8 +1169,8 @@ static AttrBuilder IdentifyValidAttributes(CallBase &CB) {
return Valid;
}
-static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap) {
- if (!UpdateReturnAttributes && !UpdateLoadMetadataDuringInlining)
+static void AddReturnAttributes(CallSite CS, ValueToValueMapTy &VMap) {
+ if (!UpdateReturnAttributes)
return;
AttrBuilder Valid = IdentifyValidAttributes(CB);
@@ -1183,32 +1179,18 @@ static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap) {
auto *CalledFunction = CB.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)
+ if (!RI || !isa<CallBase>(RI->getOperand(0)))
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 = getExpectedRV(RI->getOperand(0));
+ auto *NewRetVal = dyn_cast_or_null<CallBase>(VMap.lookup(RetVal));
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:
@@ -1236,26 +1218,10 @@ static void AddReturnAttributes(CallBase &CB, 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.
- if (auto *NewRetValCB = dyn_cast<CallBase>(NewRetVal)) {
- AttributeList AL = NewRetValCB->getAttributes();
- AttributeList NewAL =
- AL.addAttributes(Context, AttributeList::ReturnIndex, Valid);
- NewRetValCB->setAttributes(NewAL);
- } else {
- auto *NewLI = cast<LoadInst>(NewRetVal);
- if (CB.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));
- }
+ AttributeList AL = NewRetVal->getAttributes();
+ AttributeList NewAL =
+ AL.addAttributes(Context, AttributeList::ReturnIndex, Valid);
+ NewRetVal->setAttributes(NewAL);
}
}
diff --git a/llvm/test/Transforms/Inline/ret_load_metadata.ll b/llvm/test/Transforms/Inline/ret_load_metadata.ll
deleted file mode 100644
index 13dabb074017..000000000000
--- a/llvm/test/Transforms/Inline/ret_load_metadata.ll
+++ /dev/null
@@ -1,103 +0,0 @@
-; 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