[llvm] [AVR] Fix for issue #163015, occasional corruption in stack passed params (PR #164001)

Carl Peto via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 17 12:00:57 PDT 2025


https://github.com/carlos4242 updated https://github.com/llvm/llvm-project/pull/164001

>From 059e213d064c0887ef559a75544ad4e2ce5fb29d Mon Sep 17 00:00:00 2001
From: Carl Peto <carl.peto at me.com>
Date: Fri, 17 Oct 2025 20:00:25 +0100
Subject: [PATCH 1/2] Create a test for issue #163015

---
 llvm/test/CodeGen/AVR/issue-163015.ll | 34 +++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 llvm/test/CodeGen/AVR/issue-163015.ll

diff --git a/llvm/test/CodeGen/AVR/issue-163015.ll b/llvm/test/CodeGen/AVR/issue-163015.ll
new file mode 100644
index 0000000000000..60b9f9bcdbfb6
--- /dev/null
+++ b/llvm/test/CodeGen/AVR/issue-163015.ll
@@ -0,0 +1,34 @@
+; RUN: llc < %s -mtriple=avr | FileCheck -v -dump-input always %s
+
+%Tstats = type <{ i8, i8 }>
+ at ui1 = protected local_unnamed_addr global i64 zeroinitializer, align 8
+ at ui2 = protected local_unnamed_addr global i64 zeroinitializer, align 8
+ at failed = private unnamed_addr addrspace(1) constant [12 x i8] c"test failed\00"
+ at stats = external protected global %Tstats, align 1
+
+; CHECK-LABEL: main:
+define protected noundef i32 @main(i32 %0, ptr nocapture readnone %1) local_unnamed_addr addrspace(1) #0 {
+entry:
+  store i64 94, ptr @ui1, align 8
+  store i64 53, ptr @ui2, align 8
+  tail call addrspace(1) void @reportFailureWithDetails(i16 ptrtoint (ptr addrspace(1) @failed to i16), i16 11, i8 2, i16 32, ptr nocapture nonnull swiftself dereferenceable(2) @stats)
+  %11 = load i64, ptr @ui1, align 8
+  %12 = load i64, ptr @ui2, align 8
+
+; COM: CHECK: call __udivdi3
+  %15 = udiv i64 %11, %12
+
+; look for the buggy pattern where r30/r31 are being clobbered, corrupting the stack pointer
+; CHECK-NOT: std  Z+{{[1-9]+}}, r30 
+; CHECK-NOT: std  Z+{{[1-9]+}}, r31
+
+; CHECK: call expect
+  tail call addrspace(1) void @expect(i64 %15, i64 1, i16 ptrtoint (ptr addrspace(1) @failed to i16), i16 11, i8 2, i16 33)
+
+; CHECK: ret
+  ret i32 0
+}
+
+declare protected void @expect(i64, i64, i16, i16, i8, i16) local_unnamed_addr addrspace(1) #0
+declare protected void @reportFailureWithDetails(i16, i16, i8, i16, ptr nocapture swiftself dereferenceable(2)) local_unnamed_addr addrspace(1) #0
+attributes #0 = { nounwind "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }

>From aa54b9a505fd10f1e58ee2958931a25c4206cf18 Mon Sep 17 00:00:00 2001
From: Carl Peto <carl.peto at me.com>
Date: Fri, 17 Oct 2025 19:50:25 +0100
Subject: [PATCH 2/2] Corruption can occur with passing parameters on the stack
 when under register pressure.

See this forum discussion for details (or the GitHub issue) https://discourse.llvm.org/t/avr-register-allocation-issue-help-advice-with-debugging/88498/11

@aykevl @benshi001
---
 llvm/lib/Target/AVR/AVRInstrInfo.td    | 16 ++++++++++++++--
 llvm/lib/Target/AVR/AVRRegisterInfo.td | 25 +++++++++++++++++++++++++
 llvm/test/CodeGen/AVR/dynalloca.ll     | 20 ++++++++++----------
 3 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/llvm/lib/Target/AVR/AVRInstrInfo.td b/llvm/lib/Target/AVR/AVRInstrInfo.td
index 02fb905f5fb69..4a2f714278f15 100644
--- a/llvm/lib/Target/AVR/AVRInstrInfo.td
+++ b/llvm/lib/Target/AVR/AVRInstrInfo.td
@@ -1504,14 +1504,26 @@ let Defs = [SREG], hasSideEffects = 0 in
 def FRMIDX : Pseudo<(outs DLDREGS:$dst), (ins DLDREGS:$src, i16imm:$src2),
                     "frmidx\t$dst, $src, $src2", []>;
 
+// The instructions STDSPQRr and STDWSPQRr are used to store to the stack
+// frame. The most accurate implementation would be to load the SP into
+// a temporary pointer variable and then STDPtrQRr. However for efficiency,
+// we assume that R29R28 contains the current call frame pointer.
+// However in the PEI pass we sometimes rewrite a ADJCALLSTACKDOWN pseudo,
+// plus one or more STDSPQRr/STDWSPQRr pseudo instructions to use Z for a
+// stack adjustment then as a base pointer. To avoid corruption, we thus
+// specify special classes of registers, like GPR8 and DREGS, but with
+// the Z register removed, as the source/input to these instructions.
 // This pseudo is either converted to a regular store or a push which clobbers
 // SP.
-def STDSPQRr : StorePseudo<(outs), (ins memspi:$dst, GPR8:$src),
+let Defs = [SP], Uses = [SP], hasSideEffects = 0 in
+def STDSPQRr : StorePseudo<(outs), (ins memspi:$dst, GPR8NOZ:$src),
                            "stdstk\t$dst, $src", [(store i8:$src, addr:$dst)]>;
 
+// See the comment on STDSPQRr.
 // This pseudo is either converted to a regular store or a push which clobbers
 // SP.
-def STDWSPQRr : StorePseudo<(outs), (ins memspi:$dt, DREGS:$src),
+let Defs = [SP], Uses = [SP], hasSideEffects = 0 in
+def STDWSPQRr : StorePseudo<(outs), (ins memspi:$dt, DREGSNOZ:$src),
                             "stdwstk\t$dt, $src", [(store i16:$src, addr:$dt)]>;
 
 // SP read/write pseudos.
diff --git a/llvm/lib/Target/AVR/AVRRegisterInfo.td b/llvm/lib/Target/AVR/AVRRegisterInfo.td
index 182f92c684dc0..9b935b10d2b51 100644
--- a/llvm/lib/Target/AVR/AVRRegisterInfo.td
+++ b/llvm/lib/Target/AVR/AVRRegisterInfo.td
@@ -211,6 +211,31 @@ def PTRDISPREGS : RegisterClass<"AVR", [i16], 8, (add R31R30, R29R28), ptr>;
 // model this using a register class containing only the Z register.
 def ZREG : RegisterClass<"AVR", [i16], 8, (add R31R30)>;
 
+// general registers excluding Z register lo/hi, these are the only
+// registers that are always safe for STDSPQr instructions
+def GPR8NOZ : RegisterClass<"AVR", [i8], 8,
+                         (// Return value and argument registers.
+                          add R24, R25, R18, R19, R20, R21, R22, R23,
+                          // Scratch registers.
+                          R26, R27,
+                          // Callee saved registers.
+                          R28, R29, R17, R16, R15, R14, R13, R12, R11, R10,
+                          R9, R8, R7, R6, R5, R4, R3, R2, R0, R1)>;
+
+// 16-bit pair register class excluding Z register lo/hi, these are the only
+// registers that are always safe for STDWSPQr instructions
+def DREGSNOZ : RegisterClass<"AVR", [i16], 8,
+                          (// Return value and arguments.
+                           add R25R24, R19R18, R21R20, R23R22,
+                           // Scratch registers.
+                           R27R26,
+                           // Callee saved registers.
+                           R29R28, R17R16, R15R14, R13R12, R11R10, R9R8,
+                           R7R6, R5R4, R3R2, R1R0,
+                           // Pseudo regs for unaligned 16-bits
+                           R26R25, R24R23, R22R21, R20R19, R18R17, R16R15,
+                           R14R13, R12R11, R10R9)>;
+
 // Register class used for the stack read pseudo instruction.
 def GPRSP : RegisterClass<"AVR", [i16], 8, (add SP)>;
 
diff --git a/llvm/test/CodeGen/AVR/dynalloca.ll b/llvm/test/CodeGen/AVR/dynalloca.ll
index 3face71c988b0..b32910bf8a762 100644
--- a/llvm/test/CodeGen/AVR/dynalloca.ll
+++ b/llvm/test/CodeGen/AVR/dynalloca.ll
@@ -64,16 +64,16 @@ define void @dynalloca2(i16 %x) {
 ; CHECK-NEXT: out 63, r0
 ; CHECK-NEXT: out 61, {{.*}}
 ; Store values on the stack
-; CHECK: ldi r20, 0
-; CHECK: ldi r21, 0
-; CHECK: std Z+8, r21
-; CHECK: std Z+7, r20
-; CHECK: std Z+6, r21
-; CHECK: std Z+5, r20
-; CHECK: std Z+4, r21
-; CHECK: std Z+3, r20
-; CHECK: std Z+2, r21
-; CHECK: std Z+1, r20
+; CHECK: ldi [[REG1:r[0-9]+]], 0
+; CHECK: ldi [[REG2:r[0-9]+]], 0
+; CHECK: std Z+8, [[REG2]]
+; CHECK: std Z+7, [[REG1]]
+; CHECK: std Z+6, [[REG2]]
+; CHECK: std Z+5, [[REG1]]
+; CHECK: std Z+4, [[REG2]]
+; CHECK: std Z+3, [[REG1]]
+; CHECK: std Z+2, [[REG2]]
+; CHECK: std Z+1, [[REG1]]
 ; CHECK: call
 ; Call frame restore
 ; CHECK-NEXT: in r30, 61



More information about the llvm-commits mailing list