[llvm] 3fb8c5b - [X86] Fix invalid instructions on x32 with large stack frames (#124041)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 22 23:07:10 PST 2025
Author: mconst
Date: 2025-01-23T12:37:07+05:30
New Revision: 3fb8c5b43195d6e11ff0557d07e75700343d369f
URL: https://github.com/llvm/llvm-project/commit/3fb8c5b43195d6e11ff0557d07e75700343d369f
DIFF: https://github.com/llvm/llvm-project/commit/3fb8c5b43195d6e11ff0557d07e75700343d369f.diff
LOG: [X86] Fix invalid instructions on x32 with large stack frames (#124041)
`X86FrameLowering::emitSPUpdate()` assumes that 64-bit targets use a
64-bit stack pointer, but that's not true on x32.
When checking the stack pointer size, we need to look at
`Uses64BitFramePtr` rather than `Is64Bit`. This avoids generating
invalid instructions like `add esp, rcx`.
For impossibly-large stack frames (4 GiB or larger with a 32-bit stack
pointer), we were also generating invalid instructions like `mov eax,
5000000000`. The inline stack probe code already had a check for that
situation; I've moved the check into `emitSPUpdate()`, so any attempt to
allocate a 4 GiB stack frame with a 32-bit stack pointer will now trap
rather than adjusting ESP by the wrong amount. This also fixes the
"can't have 32-bit 16GB stack frame" assertion, which used to be
triggerable by user code but is now correct.
To help catch situations like this in the future, I've added
`-verify-machineinstrs` to the stack clash tests that generate large
stack frames.
This fixes the expensive-checks buildbot failure caused by #113219.
Added:
Modified:
llvm/lib/Target/X86/X86FrameLowering.cpp
llvm/test/CodeGen/X86/stack-clash-extra-huge.ll
llvm/test/CodeGen/X86/stack-clash-huge.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp
index 18de38b2d01597..47cc6a18ef8433 100644
--- a/llvm/lib/Target/X86/X86FrameLowering.cpp
+++ b/llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -253,17 +253,19 @@ void X86FrameLowering::emitSPUpdate(MachineBasicBlock &MBB,
// Rather than emit a long series of instructions for large offsets,
// load the offset into a register and do one sub/add
unsigned Reg = 0;
- unsigned Rax = (unsigned)(Is64Bit ? X86::RAX : X86::EAX);
+ unsigned Rax = (unsigned)(Uses64BitFramePtr ? X86::RAX : X86::EAX);
if (isSub && !isEAXLiveIn(MBB))
Reg = Rax;
else
- Reg = TRI->findDeadCallerSavedReg(MBB, MBBI);
+ Reg = getX86SubSuperRegister(TRI->findDeadCallerSavedReg(MBB, MBBI),
+ Uses64BitFramePtr ? 64 : 32);
- unsigned AddSubRROpc =
- isSub ? getSUBrrOpcode(Is64Bit) : getADDrrOpcode(Is64Bit);
+ unsigned AddSubRROpc = isSub ? getSUBrrOpcode(Uses64BitFramePtr)
+ : getADDrrOpcode(Uses64BitFramePtr);
if (Reg) {
- BuildMI(MBB, MBBI, DL, TII.get(getMOVriOpcode(Is64Bit, Offset)), Reg)
+ BuildMI(MBB, MBBI, DL, TII.get(getMOVriOpcode(Uses64BitFramePtr, Offset)),
+ Reg)
.addImm(Offset)
.setMIFlag(Flag);
MachineInstr *MI = BuildMI(MBB, MBBI, DL, TII.get(AddSubRROpc), StackPtr)
@@ -279,7 +281,7 @@ void X86FrameLowering::emitSPUpdate(MachineBasicBlock &MBB,
// addq %rsp, %rax
// xchg %rax, (%rsp)
// movq (%rsp), %rsp
- assert(Is64Bit && "can't have 32-bit 16GB stack frame");
+ assert(Uses64BitFramePtr && "can't have 32-bit 16GB stack frame");
BuildMI(MBB, MBBI, DL, TII.get(X86::PUSH64r))
.addReg(Rax, RegState::Kill)
.setMIFlag(Flag);
@@ -289,7 +291,8 @@ void X86FrameLowering::emitSPUpdate(MachineBasicBlock &MBB,
Offset = -(Offset - SlotSize);
else
Offset = Offset + SlotSize;
- BuildMI(MBB, MBBI, DL, TII.get(getMOVriOpcode(Is64Bit, Offset)), Rax)
+ BuildMI(MBB, MBBI, DL, TII.get(getMOVriOpcode(Uses64BitFramePtr, Offset)),
+ Rax)
.addImm(Offset)
.setMIFlag(Flag);
MachineInstr *MI = BuildMI(MBB, MBBI, DL, TII.get(X86::ADD64rr), Rax)
diff --git a/llvm/test/CodeGen/X86/stack-clash-extra-huge.ll b/llvm/test/CodeGen/X86/stack-clash-extra-huge.ll
index 59cbcd0689fbf8..b8031056fd6b0a 100644
--- a/llvm/test/CodeGen/X86/stack-clash-extra-huge.ll
+++ b/llvm/test/CodeGen/X86/stack-clash-extra-huge.ll
@@ -1,7 +1,7 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --no_x86_scrub_sp
-; RUN: llc -mtriple=x86_64-linux-android < %s | FileCheck -check-prefix=CHECK-X64 %s
-; RUN: llc -mtriple=i686-linux-android < %s | FileCheck -check-prefix=CHECK-X86 %s
-; RUN: llc -mtriple=x86_64-linux-gnux32 < %s | FileCheck -check-prefix=CHECK-X32 %s
+; RUN: llc -mtriple=x86_64-linux-android -verify-machineinstrs < %s | FileCheck -check-prefix=CHECK-X64 %s
+; RUN: llc -mtriple=i686-linux-android -verify-machineinstrs < %s | FileCheck -check-prefix=CHECK-X86 %s
+; RUN: llc -mtriple=x86_64-linux-gnux32 -verify-machineinstrs < %s | FileCheck -check-prefix=CHECK-X32 %s
define i32 @foo() local_unnamed_addr #0 {
; CHECK-X64-LABEL: foo:
@@ -66,8 +66,8 @@ define i32 @foo() local_unnamed_addr #0 {
; CHECK-X32-NEXT: movl $1, 264(%esp)
; CHECK-X32-NEXT: movl $1, 28664(%esp)
; CHECK-X32-NEXT: movl -128(%esp), %eax
-; CHECK-X32-NEXT: movabsq $4799999880, %rcx # imm = 0x11E1A2F88
-; CHECK-X32-NEXT: addq %rcx, %esp
+; CHECK-X32-NEXT: movl $4799999880, %ecx # imm = 0x11E1A2F88
+; CHECK-X32-NEXT: addl %ecx, %esp
; CHECK-X32-NEXT: .cfi_def_cfa_offset 8
; CHECK-X32-NEXT: retq
%a = alloca i32, i64 1200000000, align 16
diff --git a/llvm/test/CodeGen/X86/stack-clash-huge.ll b/llvm/test/CodeGen/X86/stack-clash-huge.ll
index 03f028dfc25067..c9990773201f0b 100644
--- a/llvm/test/CodeGen/X86/stack-clash-huge.ll
+++ b/llvm/test/CodeGen/X86/stack-clash-huge.ll
@@ -1,7 +1,7 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --no_x86_scrub_sp
-; RUN: llc -mtriple=x86_64-linux-android < %s | FileCheck -check-prefix=CHECK-X64 %s
-; RUN: llc -mtriple=i686-linux-android < %s | FileCheck -check-prefix=CHECK-X86 %s
-; RUN: llc -mtriple=x86_64-linux-gnux32 < %s | FileCheck -check-prefix=CHECK-X32 %s
+; RUN: llc -mtriple=x86_64-linux-android -verify-machineinstrs < %s | FileCheck -check-prefix=CHECK-X64 %s
+; RUN: llc -mtriple=i686-linux-android -verify-machineinstrs < %s | FileCheck -check-prefix=CHECK-X86 %s
+; RUN: llc -mtriple=x86_64-linux-gnux32 -verify-machineinstrs < %s | FileCheck -check-prefix=CHECK-X32 %s
define i32 @foo() local_unnamed_addr #0 {
; CHECK-X64-LABEL: foo:
@@ -69,7 +69,7 @@ define i32 @foo() local_unnamed_addr #0 {
; CHECK-X32-NEXT: movl $1, 28664(%esp)
; CHECK-X32-NEXT: movl -128(%esp), %eax
; CHECK-X32-NEXT: movl $2399999880, %ecx # imm = 0x8F0D1788
-; CHECK-X32-NEXT: addq %rcx, %esp
+; CHECK-X32-NEXT: addl %ecx, %esp
; CHECK-X32-NEXT: .cfi_def_cfa_offset 8
; CHECK-X32-NEXT: retq
%a = alloca i32, i64 600000000, align 16
More information about the llvm-commits
mailing list