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.

2 files changed, 9 insertions, 146 deletions

diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 9ad9f7b..92ab8f9 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 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);
   }
 }
 
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 13dabb0..0000000
--- 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}