Revert "[IRMover] Remove UB implying parameter attributes when necessary"

This reverts commit 451799bb8261bde52bbfef226d019caf1d82aa42.

Causes crashes, see D139209

diff --git a/llvm/include/llvm/IR/Attributes.h b/llvm/include/llvm/IR/Attributes.h
index fa179c9..c4e12a6 100644 (file)
--- a/llvm/include/llvm/IR/Attributes.h
+++ b/llvm/include/llvm/IR/Attributes.h
@@ -360,13 +360,6 @@ public:
   /// Return true if the attribute exists in this set.
   bool hasAttribute(StringRef Kind) const;
 
-  /// Return true if the attribute exists in this set.
-  bool hasAttribute(Attribute A) const {
-    if (A.isStringAttribute())
-      return hasAttribute(A.getKindAsString());
-    return hasAttribute(A.getKindAsEnum());
-  }
-
   /// Return the attribute object.
   Attribute getAttribute(Attribute::AttrKind Kind) const;
 

diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index fc22630..721acb1 100644 (file)
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -529,7 +529,7 @@ class IRLinker {
   void linkNamedMDNodes();
 
   ///  Update attributes while linking.
-  void updateAttributes(GlobalValue &DstGV, GlobalValue &SrcGV);
+  void updateAttributes(GlobalValue &GV);
 
 public:
   IRLinker(Module &DstM, MDMapT &SharedMDs,
@@ -641,7 +641,7 @@ Value *IRLinker::materialize(Value *V, bool ForIndirectSymbol) {
   if (ForIndirectSymbol || shouldLink(New, *SGV))
     setError(linkGlobalValueBody(*New, *SGV));
 
-  updateAttributes(*New, *SGV);
+  updateAttributes(*New);
   return New;
 }
 
@@ -1533,10 +1533,7 @@ static std::string adjustInlineAsm(const std::string &InlineAsm,
   return InlineAsm;
 }
 
-void IRLinker::updateAttributes(GlobalValue &DstGV, GlobalValue &SrcGV) {
-  auto *DstF = dyn_cast<Function>(&DstGV);
-  auto *SrcF = dyn_cast<Function>(&SrcGV);
-
+void IRLinker::updateAttributes(GlobalValue &GV) {
   /// Remove nocallback attribute while linking, because nocallback attribute
   /// indicates that the function is only allowed to jump back into caller's
   /// module only by a return or an exception. When modules are linked, this
@@ -1551,53 +1548,16 @@ void IRLinker::updateAttributes(GlobalValue &DstGV, GlobalValue &SrcGV) {
   /// participate in the LTO link, and thus ends up in the merged module
   /// containing its caller and callee, removing the attribute doesn't hurt as
   /// it has no effect on definitions in the same module.
-  if (DstF) {
-    if (!DstF->isIntrinsic())
-      DstF->removeFnAttr(llvm::Attribute::NoCallback);
+  if (auto *F = dyn_cast<Function>(&GV)) {
+    if (!F->isIntrinsic())
+      F->removeFnAttr(llvm::Attribute::NoCallback);
 
     // Remove nocallback attribute when it is on a call-site.
-    for (BasicBlock &BB : *DstF)
+    for (BasicBlock &BB : *F)
       for (Instruction &I : BB)
         if (CallBase *CI = dyn_cast<CallBase>(&I))
           CI->removeFnAttr(Attribute::NoCallback);
   }
-
-  // Fix compatibility of diverged function signatures.
-  //
-  // If `F` was a definition in its source module but is (already) a
-  // declaration in the destination module, then the signatures of the two
-  // functions may have diverged (even if they were originally idential) due to
-  // optimizations performed on the source module.
-  //
-  // See the test in `FunctionImport/attr_fixup_dae_noundef.ll` for an example
-  // based on how Dead Argument Elimination can remove `noundef` from function
-  // arguments.
-  //
-  // The code below handles _known_ incompatibilities. Others may exist.
-  if (DstF && SrcF && DstF->isDeclaration() && !SrcF->isDeclaration()) {
-    assert(DstF->arg_size() == SrcF->arg_size() &&
-           "Dst and Src should have the same signature.");
-
-    // Remove UB implying attributes present on `Dst`'s arguments but not
-    // `Src`'s.
-    AttributeMask UBImplyingAttributes =
-        AttributeFuncs::getUBImplyingAttributes();
-    for (size_t ArgI = 0; ArgI < DstF->arg_size(); ArgI++) {
-      AttributeSet DstAttrs = DstF->getAttributes().getParamAttrs(ArgI);
-      AttributeSet SrcAttrs = SrcF->getAttributes().getParamAttrs(ArgI);
-      AttributeMask ToRemove;
-
-      for (auto &Attr : DstAttrs) {
-        if (SrcAttrs.hasAttribute(Attr))
-          continue;
-
-        if (UBImplyingAttributes.contains(Attr))
-          ToRemove.addAttribute(Attr);
-      }
-
-      DstF->removeParamAttrs(ArgI, ToRemove);
-    }
-  }
 }
 
 Error IRLinker::run() {

diff --git a/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll b/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll
deleted file mode 100644 (file)
index c8e3121..0000000
--- a/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll
+++ /dev/null
@@ -1,17 +0,0 @@
-; This file contains the post-"deadargelim" IR.
-
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-define void @outer(i32 noundef %arg) {
-  ; The parameter was originally `noundef %arg`, changed to `poison` by "deadargelim".
-  call void @inner(i32 poison)
-  ret void
-}
-
-; %arg was originally `noundef`, removed by "deadargelim".
-define void @inner(i32 %arg) #0 {
-  ret void
-}
-
-attributes #0 = { noinline }

diff --git a/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll b/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll
deleted file mode 100644 (file)
index fa43f98..0000000
--- a/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll
+++ /dev/null
@@ -1,34 +0,0 @@
-; Test to ensure that if a definition is imported, already-present declarations
-; are updated as necessary: Definitions from the same module may be optimized
-; together. Thus care must be taken when importing only a subset of the
-; definitions from a module (because other referenced definitions from that
-; module may have been changed by the optimizer and may no longer match
-; declarations already present in the module being imported into).
-
-; Generate bitcode and index, and run the function import.
-; `Inputs/attr_fixup_dae_noundef.ll` contains the post-"Dead Argument Elimination" IR, which
-; removed the `noundef` from `@inner`.
-; RUN: opt -module-summary %p/Inputs/attr_fixup_dae_noundef.ll -o %t.inputs.attr_fixup_dae_noundef.bc
-; RUN: opt -module-summary %s -o %t.main.bc
-; RUN: llvm-lto -thinlto -o %t.summary %t.main.bc %t.inputs.attr_fixup_dae_noundef.bc
-; RUN: opt -passes=function-import -summary-file %t.summary.thinlto.bc %t.main.bc -S 2>&1 \
-; RUN:   | FileCheck %s
-
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-define void @main()  {
-  call void @outer(i32 noundef 1)
-  call void @inner(i32 noundef 1)
-  ret void
-}
-
-; Because `@inner` is `noinline`, it should not get imported. However, the
-; `noundef` should be removed.
-; CHECK: declare void @inner(i32)
-declare void @inner(i32 noundef)
-
-; `@outer` should get imported.
-; CHECK: define available_externally void @outer(i32 noundef %arg)
-; CHECK-NEXT: call void @inner(i32 poison)
-declare void @outer(i32 noundef)