Revert "[InlineFunction] Update metadata on loads that are return values"

This reverts commit 1d0f757904 because of
https://bugs.llvm.org/show_bug.cgi?id=45590. Needs investigation.
This commit is contained in:
Anna Thomas 2020-04-17 17:11:21 -04:00
parent 7cb1aa9d93
commit ef49b1d97e
2 changed files with 9 additions and 146 deletions

View File

@ -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 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.
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);
}
}

View File

@ -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}