[llvm] [MergeFunc] Fix poison flag merging for varargs functions (PR #223347)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 14 02:52:00 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Nikita Popov (nikic)
<details>
<summary>Changes</summary>
This is a followup to https://github.com/llvm/llvm-project/pull/220015. writeThunkOrAliasIfNeeded() has an early exit for the case where none of erase/thunk/alias apply, and we failed to merge the poison flags in that case. I've opted to fix this by moving the annotation merging logic out of writeThunkOrAliasIfNeeded() entirely, because I don't think it logically fits there (it's not related to thunk/alias creation at all).
This is the minimal fix to address the miscompile. There is a potential larger change we could make here, which is to avoid "merging" the functions in this case in the first place. We don't actually merge the functions here, but just rewrite all uses to one of them. This seems kind of dubious to me, though I could see an argument that this can still be beneficial in conjunction with --gc-sections (i.e. linker-level dead function elimination).
---
Full diff: https://github.com/llvm/llvm-project/pull/223347.diff
2 Files Affected:
- (modified) llvm/lib/Transforms/IPO/MergeFunctions.cpp (+12-18)
- (modified) llvm/test/Transforms/MergeFunc/flags.ll (+24)
``````````diff
diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index 0101b469b215e..9bb18c2f3cb04 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -307,11 +307,7 @@ class MergeFunctions {
// If needed, replace G with an alias to F if possible, or a thunk to F if
// profitable. Returns false if neither is the case. If \p G is not needed
// (i.e. it is discardable and not used), \p G is removed directly.
- // If \p MergeAnnotations is true, annotations on G such as profiling
- // information and poison-generating flags are merged into F before G is
- // erased or rewritten.
- bool writeThunkOrAliasIfNeeded(Function *F, Function *G,
- bool MergeAnnotations);
+ bool writeThunkOrAliasIfNeeded(Function *F, Function *G);
/// Replace function F with function G in the function tree.
void replaceFunctionInTree(const FunctionNode &FN, Function *G);
@@ -914,8 +910,7 @@ static void mergeEntryCountsAndImportsInto(Function &F, Function &G) {
F.setEntryCount(Sum, AllImports.empty() ? nullptr : &AllImports);
}
-bool MergeFunctions::writeThunkOrAliasIfNeeded(Function *F, Function *G,
- bool MergeAnnotations) {
+bool MergeFunctions::writeThunkOrAliasIfNeeded(Function *F, Function *G) {
bool ShouldErase =
G->isDiscardableIfUnused() && G->use_empty() && !MergeFunctionsPDI;
bool ShouldAlias = canCreateAliasFor(G);
@@ -924,11 +919,6 @@ bool MergeFunctions::writeThunkOrAliasIfNeeded(Function *F, Function *G,
if (!ShouldErase && !ShouldAlias && !ShouldThunk)
return false;
- if (MergeAnnotations) {
- mergeInstrAnnotations(F, G);
- mergeEntryCountsAndImportsInto(*F, *G);
- }
-
if (ShouldErase) {
G->eraseFromParent();
return true;
@@ -1164,13 +1154,16 @@ void MergeFunctions::mergeTwoFunctions(Function *F, Function *G) {
const MaybeAlign NewFAlign = NewF->getAlign();
const MaybeAlign GAlign = G->getAlign();
- // Merge !prof, while G still has its body.
- writeThunkOrAliasIfNeeded(F, G, /*MergeAnnotations=*/true);
+ // Merge annotations, while G still has its body.
+ mergeInstrAnnotations(F, G);
+ mergeEntryCountsAndImportsInto(*F, *G);
+
+ writeThunkOrAliasIfNeeded(F, G);
if (FEntryCount)
NewF->setEntryCount(*FEntryCount);
// NewF becomes thunk/alias to the shared body F, it has no annotations to
// be merged.
- writeThunkOrAliasIfNeeded(F, NewF, /*MergeAnnotations=*/false);
+ writeThunkOrAliasIfNeeded(F, NewF);
if (NewFAlign || GAlign)
F->setAlignment(std::max(NewFAlign.valueOrOne(), GAlign.valueOrOne()));
@@ -1200,18 +1193,19 @@ void MergeFunctions::mergeTwoFunctions(Function *F, Function *G) {
}
}
+ mergeInstrAnnotations(F, G);
+ mergeEntryCountsAndImportsInto(*F, *G);
+
// If G was internal then we may have replaced all uses of G with F. If so,
// stop here and delete G. There's no need for a thunk. (See note on
// MergeFunctionsPDI above).
if (G->isDiscardableIfUnused() && G->use_empty() && !MergeFunctionsPDI) {
- mergeInstrAnnotations(F, G);
- mergeEntryCountsAndImportsInto(*F, *G);
G->eraseFromParent();
++NumFunctionsMerged;
return;
}
- if (writeThunkOrAliasIfNeeded(F, G, /*MergeAnnotations=*/true))
+ if (writeThunkOrAliasIfNeeded(F, G))
++NumFunctionsMerged;
}
}
diff --git a/llvm/test/Transforms/MergeFunc/flags.ll b/llvm/test/Transforms/MergeFunc/flags.ll
index 17907ab1f227a..ab02205eb14f2 100644
--- a/llvm/test/Transforms/MergeFunc/flags.ll
+++ b/llvm/test/Transforms/MergeFunc/flags.ll
@@ -61,6 +61,26 @@ define internal float @fn_fadd_ninf_reassoc(float %a) {
ret float %add
}
+define i32 @fn_vararg_add_nsw(i32 %a, ...) {
+; CHECK-LABEL: define i32 @fn_vararg_add_nsw(
+; CHECK-SAME: i32 [[A:%.*]], ...) {
+; CHECK-NEXT: [[ADD:%.*]] = add i32 [[A]], 1
+; CHECK-NEXT: ret i32 [[ADD]]
+;
+ %add = add nsw i32 %a, 1
+ ret i32 %add
+}
+
+define i32 @fn_vararg_add_wrap(i32 %a, ...) {
+; CHECK-LABEL: define i32 @fn_vararg_add_wrap(
+; CHECK-SAME: i32 [[A:%.*]], ...) {
+; CHECK-NEXT: [[ADD:%.*]] = add i32 [[A]], 1
+; CHECK-NEXT: ret i32 [[ADD]]
+;
+ %add = add i32 %a, 1
+ ret i32 %add
+}
+
define void @calls(i32 %x, ptr %p, float %f) {
; CHECK-LABEL: define void @calls(
; CHECK-SAME: i32 [[X:%.*]], ptr [[P:%.*]], float [[F:%.*]]) {
@@ -72,6 +92,8 @@ define void @calls(i32 %x, ptr %p, float %f) {
; CHECK-NEXT: [[TMP6:%.*]] = call ptr @fn_gep_inbounds2(ptr [[P]])
; CHECK-NEXT: [[TMP7:%.*]] = call float @fn_fadd_ninf_nnan(float [[F]])
; CHECK-NEXT: [[TMP8:%.*]] = call float @fn_fadd_ninf_nnan(float [[F]])
+; CHECK-NEXT: [[TMP9:%.*]] = call i32 (i32, ...) @fn_vararg_add_nsw(i32 [[X]])
+; CHECK-NEXT: [[TMP10:%.*]] = call i32 (i32, ...) @fn_vararg_add_nsw(i32 [[X]])
; CHECK-NEXT: ret void
;
call i32 @fn_add_nuw_nsw(i32 %x)
@@ -82,5 +104,7 @@ define void @calls(i32 %x, ptr %p, float %f) {
call ptr @fn_gep_nuw(ptr %p)
call float @fn_fadd_ninf_nnan(float %f)
call float @fn_fadd_ninf_reassoc(float %f)
+ call i32 (i32, ...) @fn_vararg_add_nsw(i32 %x)
+ call i32 (i32, ...) @fn_vararg_add_wrap(i32 %x)
ret void
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/223347
More information about the llvm-commits
mailing list