[llvm] [X86] Don't fold loads from non-fixed stack objects into tail calls (PR #221243)
Akash Manna via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 7 01:29:45 PDT 2026
https://github.com/akash-manna-sky updated https://github.com/llvm/llvm-project/pull/221243
>From 93180cf2206c9f037962bff11155ac57aa0169fc Mon Sep 17 00:00:00 2001
From: Akash Manna <akash.manna.mymail at gmail.com>
Date: Fri, 4 Sep 2026 20:29:42 +0530
Subject: [PATCH 1/6] [X86] Don't fold loads from non-fixed stack objects into
tail calls
A tail call jump executes after the epilogue, so a memory operand can
only address fixed stack objects. Folding a callee load from a local
into TCRETURNmi/TCRETURNmi64 breaks once the frame needs dynamic
realignment, and that requirement may only appear after the sibcall
decision: a legalization temporary or a spill slot can raise the frame
alignment. PEI then hits "Return instruction can only reference SP
relative frame objects".
Reject such loads in checkTCRetEnoughRegs, which gates both the fold
patterns and the callee-load hoisting in PreprocessISelDAG, so the
callee is loaded into a register before the epilogue instead.
Fixes #216504
---
llvm/lib/Target/X86/X86ISelDAGToDAG.cpp | 35 ++++++++++++++++++++++---
llvm/test/CodeGen/X86/pr216504.ll | 31 ++++++++++++++++++++++
2 files changed, 62 insertions(+), 4 deletions(-)
create mode 100644 llvm/test/CodeGen/X86/pr216504.ll
diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
index 779e4dfce513a..20307d70fb3a4 100644
--- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -3682,7 +3682,37 @@ static bool mayUseCarryFlag(X86::CondCode CC) {
return true;
}
+/// Return true if \p Addr may be matched with a non-fixed frame index as base.
+static bool addrMayUseNonFixedFrameIndex(SDValue Addr,
+ const MachineFrameInfo &MFI,
+ unsigned Depth = 0) {
+ if (auto *FI = dyn_cast<FrameIndexSDNode>(Addr))
+ return !MFI.isFixedObjectIndex(FI->getIndex());
+ if (Depth >= SelectionDAG::MaxRecursionDepth)
+ return false;
+ switch (Addr.getOpcode()) {
+ case ISD::ADD:
+ case ISD::OR:
+ case ISD::XOR:
+ return addrMayUseNonFixedFrameIndex(Addr.getOperand(0), MFI, Depth + 1) ||
+ addrMayUseNonFixedFrameIndex(Addr.getOperand(1), MFI, Depth + 1);
+ case ISD::SUB:
+ return addrMayUseNonFixedFrameIndex(Addr.getOperand(0), MFI, Depth + 1);
+ default:
+ return false;
+ }
+}
+
bool X86DAGToDAGISel::checkTCRetEnoughRegs(SDNode *N) const {
+ assert(N->getOpcode() == X86ISD::TC_RETURN);
+ // X86tcret args: (*chain, ptr, imm, regs..., glue)
+ auto *Load = cast<LoadSDNode>(N->getOperand(1));
+
+ // The tail call executes after the epilogue, where only fixed stack objects
+ // can still be addressed (the stack may end up realigned).
+ if (addrMayUseNonFixedFrameIndex(Load->getBasePtr(), MF->getFrameInfo()))
+ return false;
+
// Check that there is enough volatile registers to load the callee address.
const X86RegisterInfo *RI = Subtarget->getRegisterInfo();
@@ -3711,13 +3741,10 @@ bool X86DAGToDAGISel::checkTCRetEnoughRegs(SDNode *N) const {
// The load's base and index need up to two registers.
unsigned LoadGPRs = 2;
- assert(N->getOpcode() == X86ISD::TC_RETURN);
- // X86tcret args: (*chain, ptr, imm, regs..., glue)
-
if (Subtarget->is32Bit()) {
// FIXME: This was carried from X86tcret_1reg which was used for 32-bit,
// but it could apply to 64-bit too.
- const SDValue &BasePtr = cast<LoadSDNode>(N->getOperand(1))->getBasePtr();
+ const SDValue &BasePtr = Load->getBasePtr();
if (isa<FrameIndexSDNode>(BasePtr)) {
LoadGPRs -= 2; // Base is fixed index off ESP; no regs needed.
} else if (BasePtr.getOpcode() == X86ISD::Wrapper &&
diff --git a/llvm/test/CodeGen/X86/pr216504.ll b/llvm/test/CodeGen/X86/pr216504.ll
new file mode 100644
index 0000000000000..43dbac2e5b2b7
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr216504.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+avx2 -verify-machineinstrs | FileCheck %s --check-prefix=X64
+; RUN: llc < %s -mtriple=i686-unknown-linux-gnu -mattr=+avx2 -verify-machineinstrs | FileCheck %s --check-prefix=X86
+
+; The variable-index extract needs a 32-byte aligned stack temporary, so the
+; stack is realigned. The callee load from a local must not be folded into the
+; tail call, which executes after the epilogue.
+
+ at g2 = global i16 0, align 2
+
+define void @f25() nounwind {
+; X64-LABEL: f25:
+; X64: andq $-32, %rsp
+; X64: movq {{[0-9]*}}(%rsp), %[[FP:r[a-z0-9]+]]
+; X64-NOT: jmpq *{{.*}}(%rsp)
+; X64: jmpq *%[[FP]]
+;
+; X86-LABEL: f25:
+; X86: andl $-32, %esp
+; X86: movl {{[0-9]*}}(%esp), %[[FP:e[a-z]+]]
+; X86-NOT: jmpl *{{.*}}(%esp)
+; X86: jmpl *%[[FP]]
+entry:
+ %fp5 = alloca ptr, align 8
+ %0 = load i16, ptr @g2, align 2
+ %vecext = extractelement <8 x i32> <i32 60, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0>, i16 %0
+ %conv = trunc i32 %vecext to i16
+ store i16 %conv, ptr @g2, align 2
+ %fp = load volatile ptr, ptr %fp5, align 8
+ tail call void %fp()
+ ret void
+}
>From 7e67a8961a3bf8b4d5f9a285e95d2ce0c34ebaa9 Mon Sep 17 00:00:00 2001
From: Akash Manna <akash.manna.mymail at gmail.com>
Date: Fri, 4 Sep 2026 21:37:06 +0530
Subject: [PATCH 2/6] [X86] Include MachineFrameInfo.h in X86ISelDAGToDAG.cpp
The file only had a forward declaration of MachineFrameInfo via
PseudoSourceValue.h, which is not enough for isFixedObjectIndex().
---
llvm/lib/Target/X86/X86ISelDAGToDAG.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
index 20307d70fb3a4..134d84f51914c 100644
--- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -16,6 +16,7 @@
#include "X86Subtarget.h"
#include "X86TargetMachine.h"
#include "llvm/ADT/Statistic.h"
+#include "llvm/CodeGen/MachineFrameInfo.h"
#include "llvm/CodeGen/MachineModuleInfo.h"
#include "llvm/CodeGen/SelectionDAGISel.h"
#include "llvm/Config/llvm-config.h"
>From 612b1c7815514f5985e29b5857027cd6be4bd729 Mon Sep 17 00:00:00 2001
From: Akash Manna <akash.manna.mymail at gmail.com>
Date: Sat, 5 Sep 2026 20:02:49 +0530
Subject: [PATCH 3/6] [X86] Autogenerate checks in pr216504.ll
Use full assembly checks so the ordering of the volatile callee load
relative to the preceding store is visible in the test.
---
llvm/test/CodeGen/X86/pr216504.ll | 42 +++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 8 deletions(-)
diff --git a/llvm/test/CodeGen/X86/pr216504.ll b/llvm/test/CodeGen/X86/pr216504.ll
index 43dbac2e5b2b7..6cf9609526088 100644
--- a/llvm/test/CodeGen/X86/pr216504.ll
+++ b/llvm/test/CodeGen/X86/pr216504.ll
@@ -1,3 +1,4 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+avx2 -verify-machineinstrs | FileCheck %s --check-prefix=X64
; RUN: llc < %s -mtriple=i686-unknown-linux-gnu -mattr=+avx2 -verify-machineinstrs | FileCheck %s --check-prefix=X86
@@ -9,16 +10,41 @@
define void @f25() nounwind {
; X64-LABEL: f25:
-; X64: andq $-32, %rsp
-; X64: movq {{[0-9]*}}(%rsp), %[[FP:r[a-z0-9]+]]
-; X64-NOT: jmpq *{{.*}}(%rsp)
-; X64: jmpq *%[[FP]]
+; X64: # %bb.0: # %entry
+; X64-NEXT: pushq %rbp
+; X64-NEXT: movq %rsp, %rbp
+; X64-NEXT: andq $-32, %rsp
+; X64-NEXT: subq $96, %rsp
+; X64-NEXT: movq g2 at GOTPCREL(%rip), %rax
+; X64-NEXT: movzwl (%rax), %ecx
+; X64-NEXT: andl $7, %ecx
+; X64-NEXT: vmovss {{.*#+}} xmm0 = [60,0,0,0]
+; X64-NEXT: vmovaps %ymm0, {{[0-9]+}}(%rsp)
+; X64-NEXT: movl 32(%rsp,%rcx,4), %ecx
+; X64-NEXT: movw %cx, (%rax)
+; X64-NEXT: movq {{[0-9]+}}(%rsp), %rax
+; X64-NEXT: movq %rbp, %rsp
+; X64-NEXT: popq %rbp
+; X64-NEXT: vzeroupper
+; X64-NEXT: jmpq *%rax # TAILCALL
;
; X86-LABEL: f25:
-; X86: andl $-32, %esp
-; X86: movl {{[0-9]*}}(%esp), %[[FP:e[a-z]+]]
-; X86-NOT: jmpl *{{.*}}(%esp)
-; X86: jmpl *%[[FP]]
+; X86: # %bb.0: # %entry
+; X86-NEXT: pushl %ebp
+; X86-NEXT: movl %esp, %ebp
+; X86-NEXT: andl $-32, %esp
+; X86-NEXT: subl $96, %esp
+; X86-NEXT: movzwl g2, %eax
+; X86-NEXT: andl $7, %eax
+; X86-NEXT: vmovss {{.*#+}} xmm0 = [60,0,0,0]
+; X86-NEXT: vmovaps %ymm0, {{[0-9]+}}(%esp)
+; X86-NEXT: movl 32(%esp,%eax,4), %eax
+; X86-NEXT: movw %ax, g2
+; X86-NEXT: movl {{[0-9]+}}(%esp), %eax
+; X86-NEXT: movl %ebp, %esp
+; X86-NEXT: popl %ebp
+; X86-NEXT: vzeroupper
+; X86-NEXT: jmpl *%eax # TAILCALL
entry:
%fp5 = alloca ptr, align 8
%0 = load i16, ptr @g2, align 2
>From a9cb8d45fb3a8e2dc4f0b17f53faadda567d8d9c Mon Sep 17 00:00:00 2001
From: Akash Manna <akash.manna.mymail at gmail.com>
Date: Sun, 6 Sep 2026 00:17:35 +0530
Subject: [PATCH 4/6] [X86] Be conservative at max depth in
addrMayUseNonFixedFrameIndex
Returning true when the walk gives up only costs a register load; the
default case stays false since only add-like nodes and the LHS of a SUB
can fold a frame index into the address base.
---
llvm/lib/Target/X86/X86ISelDAGToDAG.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
index 134d84f51914c..98b3a93f5b6c9 100644
--- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -3690,7 +3690,7 @@ static bool addrMayUseNonFixedFrameIndex(SDValue Addr,
if (auto *FI = dyn_cast<FrameIndexSDNode>(Addr))
return !MFI.isFixedObjectIndex(FI->getIndex());
if (Depth >= SelectionDAG::MaxRecursionDepth)
- return false;
+ return true;
switch (Addr.getOpcode()) {
case ISD::ADD:
case ISD::OR:
@@ -3700,6 +3700,8 @@ static bool addrMayUseNonFixedFrameIndex(SDValue Addr,
case ISD::SUB:
return addrMayUseNonFixedFrameIndex(Addr.getOperand(0), MFI, Depth + 1);
default:
+ // Only add-like nodes and the LHS of a SUB can fold a frame index into the
+ // base; anything else is matched as a register or symbol base.
return false;
}
}
>From e18d0820c73782747015dca7fa487d75260a0fff Mon Sep 17 00:00:00 2001
From: Akash Manna <akash.manna.mymail at gmail.com>
Date: Sun, 6 Sep 2026 07:50:06 +0530
Subject: [PATCH 5/6] [X86] Comment the depth cutoff in
`addrMayUseNonFixedFrameIndex`
---
llvm/lib/Target/X86/X86ISelDAGToDAG.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
index 98b3a93f5b6c9..825723e2c9086 100644
--- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -3689,6 +3689,7 @@ static bool addrMayUseNonFixedFrameIndex(SDValue Addr,
unsigned Depth = 0) {
if (auto *FI = dyn_cast<FrameIndexSDNode>(Addr))
return !MFI.isFixedObjectIndex(FI->getIndex());
+ // Assume the worst if we can't see the whole address expression.
if (Depth >= SelectionDAG::MaxRecursionDepth)
return true;
switch (Addr.getOpcode()) {
>From a803d144be1ceb1077fc2c38361c4062d41fc197 Mon Sep 17 00:00:00 2001
From: Akash Manna <akash.manna.mymail at gmail.com>
Date: Mon, 7 Sep 2026 13:58:48 +0530
Subject: [PATCH 6/6] [X86] Reuse `BasePtr` in `checkTCRetEnoughRegs`
---
llvm/lib/Target/X86/X86ISelDAGToDAG.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
index 825723e2c9086..4478930016f63 100644
--- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -3710,11 +3710,11 @@ static bool addrMayUseNonFixedFrameIndex(SDValue Addr,
bool X86DAGToDAGISel::checkTCRetEnoughRegs(SDNode *N) const {
assert(N->getOpcode() == X86ISD::TC_RETURN);
// X86tcret args: (*chain, ptr, imm, regs..., glue)
- auto *Load = cast<LoadSDNode>(N->getOperand(1));
+ const SDValue &BasePtr = cast<LoadSDNode>(N->getOperand(1))->getBasePtr();
// The tail call executes after the epilogue, where only fixed stack objects
// can still be addressed (the stack may end up realigned).
- if (addrMayUseNonFixedFrameIndex(Load->getBasePtr(), MF->getFrameInfo()))
+ if (addrMayUseNonFixedFrameIndex(BasePtr, MF->getFrameInfo()))
return false;
// Check that there is enough volatile registers to load the callee address.
@@ -3748,7 +3748,6 @@ bool X86DAGToDAGISel::checkTCRetEnoughRegs(SDNode *N) const {
if (Subtarget->is32Bit()) {
// FIXME: This was carried from X86tcret_1reg which was used for 32-bit,
// but it could apply to 64-bit too.
- const SDValue &BasePtr = Load->getBasePtr();
if (isa<FrameIndexSDNode>(BasePtr)) {
LoadGPRs -= 2; // Base is fixed index off ESP; no regs needed.
} else if (BasePtr.getOpcode() == X86ISD::Wrapper &&
More information about the llvm-commits
mailing list