[llvm-branch-commits] [llvm] release/22.x: [IRMover] Use signature for exact definition (#177381) (PR #178378)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-lto

Author: None (llvmbot)

<details>
<summary>Changes</summary>

Backport f2deb793af7b9543180bba4b9402dcc7cc7a0c2e

Requested by: @<!-- -->nikic

---
Full diff: https://github.com/llvm/llvm-project/pull/178378.diff


3 Files Affected:

- (modified) llvm/lib/Linker/IRMover.cpp (+10) 
- (added) llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll (+19) 
- (added) llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll (+38) 


``````````diff
diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index f215f39f41bfb..03c807ead9dfd 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -943,6 +943,16 @@ Expected<Constant *> IRLinker::linkGlobalValueProto(GlobalValue *SGV,
   GlobalValue *NewGV;
   if (DGV && !ShouldLink) {
     NewGV = DGV;
+
+    // If the source is an exact definition, optimizations may have modified
+    // its attributes, e.g. by dropping noundef attributes when replacing
+    // arguments with poison. In this case, it is important for correctness
+    // that we use the signature from the exact definition.
+    if (isa<Function>(SGV) && DGV->isDeclaration() &&
+        SGV->hasExactDefinition() && !DoneLinkingBodies) {
+      NewGV = copyGlobalValueProto(SGV, /*ForDefinition=*/false);
+      NeedsRenaming = true;
+    }
   } else {
     // If we are done linking global value bodies (i.e. we are performing
     // metadata linking), don't link in the global value due to this
diff --git a/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll b/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll
new file mode 100644
index 0000000000000..9fd8665ee1922
--- /dev/null
+++ b/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll
@@ -0,0 +1,19 @@
+; This file contains the post-"deadargelim" IR.
+
+define void @outer(i32 noundef %arg) {
+  ; The parameter was originally `noundef %arg`, changed to `poison` by "deadargelim".
+  call void @inner(i32 poison)
+  call void @inner2(i32 poison)
+  ret void
+}
+
+; %arg was originally `noundef`, removed by "deadargelim".
+define void @inner(i32 %arg) #0 {
+  ret void
+}
+
+define void @inner2(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
new file mode 100644
index 0000000000000..d1679860fce86
--- /dev/null
+++ b/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll
@@ -0,0 +1,38 @@
+; Test to ensure that if an exact definition is imported, it is used in favor
+; of an already-present declaration. Exact 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
+
+define void @main()  {
+  call void @outer(i32 noundef 1)
+  call void @inner(i32 noundef 1)
+  ; This call is UB due to signature mismatch with the actual definition.
+  ; Make sure it does not lead to a crash.
+  call void @inner2()
+  ret void
+}
+
+; `@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)
+
+; Because `@inner` is `noinline`, the definition should not be important.
+; However, we should create a new declaration from the definition, which does
+; not have the `noundef` attribute.
+; CHECK: declare void @inner(i32)
+declare void @inner(i32 noundef)
+
+; CHECK: declare void @inner2(i32)
+declare void @inner2()

``````````

</details>


https://github.com/llvm/llvm-project/pull/178378