[llvm] [IRMover] Use signature for exact definition (PR #177381)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 22 07:32:31 PST 2026
https://github.com/nikic created https://github.com/llvm/llvm-project/pull/177381
It is possible for optimizations to modify attributes on exact definitions. In particular, DeadArgumentElimination may find that a certain argument is dead, and replace arguments in calls with `poison`. This requires dropping the `noundef` attribute on the argument.
When ThinLTO import is performed, the destination module already has a declaration for the function, and the definition is not important (e.g. because it is noinline), we currently simply retain the original declaration. This is incorrect if call with poison arguments were important, as the calls become immediate UB.
There was a previous attempt to address this in https://reviews.llvm.org/D139209. What that patch did was to fix up the attributes of the declaration after the fact, dropping UB implying attributes that are not present on the definition. It was reverted because it made an incorrect assumption that the signature between the declaration and definition must match.
In this PR, I propose to fix the issue in a different way: If the source module holds an exact definition (which are the ones that can be, in limited ways, modified by optimizations), then even if we don't import the definition, we should still import a declaration based on it. This ensures that we respect any dropped attributes. (It's probably also useful for optimization purposes, because we also see more inferred attributes.)
Fixes https://github.com/llvm/llvm-project/issues/58976.
>From 0c20860beb0aa0256a06aafc69301a6f7eb93cc1 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Thu, 22 Jan 2026 14:56:04 +0100
Subject: [PATCH 1/3] Add tests from D139209
---
.../Inputs/attr_fixup_dae_noundef.ll | 14 +++++++++
.../FunctionImport/attr_fixup_dae_noundef.ll | 31 +++++++++++++++++++
2 files changed, 45 insertions(+)
create mode 100644 llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll
create mode 100644 llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll
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..ae2816ce3c18a
--- /dev/null
+++ b/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll
@@ -0,0 +1,14 @@
+; 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)
+ 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
new file mode 100644
index 0000000000000..744b69167ab9b
--- /dev/null
+++ b/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll
@@ -0,0 +1,31 @@
+; 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
+
+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)
>From bdd913b72ea10593b21bd01e887c316604750df2 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Thu, 22 Jan 2026 16:09:07 +0100
Subject: [PATCH 2/3] Import declaration from exact definition
---
llvm/lib/Linker/IRMover.cpp | 10 +++++++++
.../FunctionImport/attr_fixup_dae_noundef.ll | 21 ++++++++++---------
2 files changed, 21 insertions(+), 10 deletions(-)
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/attr_fixup_dae_noundef.ll b/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll
index 744b69167ab9b..165ff711a3206 100644
--- a/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll
+++ b/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll
@@ -1,8 +1,8 @@
-; 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
+; 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.
@@ -20,12 +20,13 @@ define void @main() {
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)
+
+; 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)
>From 21caaa79b7504a622be17ceed397efe261d9522d Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Thu, 22 Jan 2026 16:22:09 +0100
Subject: [PATCH 3/3] Add test with signature mismatch
---
.../FunctionImport/Inputs/attr_fixup_dae_noundef.ll | 5 +++++
.../Transforms/FunctionImport/attr_fixup_dae_noundef.ll | 6 ++++++
2 files changed, 11 insertions(+)
diff --git a/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll b/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll
index ae2816ce3c18a..9fd8665ee1922 100644
--- a/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll
+++ b/llvm/test/Transforms/FunctionImport/Inputs/attr_fixup_dae_noundef.ll
@@ -3,6 +3,7 @@
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
}
@@ -11,4 +12,8 @@ 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
index 165ff711a3206..d1679860fce86 100644
--- a/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll
+++ b/llvm/test/Transforms/FunctionImport/attr_fixup_dae_noundef.ll
@@ -17,6 +17,9 @@
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
}
@@ -30,3 +33,6 @@ declare void @outer(i32 noundef)
; not have the `noundef` attribute.
; CHECK: declare void @inner(i32)
declare void @inner(i32 noundef)
+
+; CHECK: declare void @inner2(i32)
+declare void @inner2()
More information about the llvm-commits
mailing list