From 059e213d064c0887ef559a75544ad4e2ce5fb29d Mon Sep 17 00:00:00 2001 From: Carl Peto Date: Fri, 17 Oct 2025 20:00:25 +0100 Subject: [PATCH 1/4] 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 }> +@ui1 = protected local_unnamed_addr global i64 zeroinitializer, align 8 +@ui2 = protected local_unnamed_addr global i64 zeroinitializer, align 8 +@failed = private unnamed_addr addrspace(1) constant [12 x i8] c"test failed\00" +@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 Date: Fri, 17 Oct 2025 19:50:25 +0100 Subject: [PATCH 2/4] 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 From f61df656aef9fd331fd13ddedc51d3de72908951 Mon Sep 17 00:00:00 2001 From: Carl Peto Date: Sat, 18 Oct 2025 08:04:23 +0100 Subject: [PATCH 3/4] requested fixes --- llvm/test/CodeGen/AVR/issue-163015.ll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/test/CodeGen/AVR/issue-163015.ll b/llvm/test/CodeGen/AVR/issue-163015.ll index 60b9f9bcdbfb6..06cbea62340f0 100644 --- a/llvm/test/CodeGen/AVR/issue-163015.ll +++ b/llvm/test/CodeGen/AVR/issue-163015.ll @@ -1,4 +1,4 @@ -; RUN: llc < %s -mtriple=avr | FileCheck -v -dump-input always %s +; RUN: llc < %s -mtriple=avr | FileCheck %s %Tstats = type <{ i8, i8 }> @ui1 = protected local_unnamed_addr global i64 zeroinitializer, align 8 @@ -7,7 +7,7 @@ @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 { +define protected noundef i32 @main(i32 %0, ptr nocapture readnone %1) addrspace(1) #0 { entry: store i64 94, ptr @ui1, align 8 store i64 53, ptr @ui2, align 8 From 445f05f053b136a6f265e5733f06d47544ccf927 Mon Sep 17 00:00:00 2001 From: Carl Peto Date: Sat, 18 Oct 2025 09:43:48 +0100 Subject: [PATCH 4/4] - requested fixes --- llvm/test/CodeGen/AVR/issue-163015.ll | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/llvm/test/CodeGen/AVR/issue-163015.ll b/llvm/test/CodeGen/AVR/issue-163015.ll index 06cbea62340f0..6c4dc518fed2f 100644 --- a/llvm/test/CodeGen/AVR/issue-163015.ll +++ b/llvm/test/CodeGen/AVR/issue-163015.ll @@ -1,17 +1,16 @@ ; RUN: llc < %s -mtriple=avr | FileCheck %s -%Tstats = type <{ i8, i8 }> @ui1 = protected local_unnamed_addr global i64 zeroinitializer, align 8 @ui2 = protected local_unnamed_addr global i64 zeroinitializer, align 8 @failed = private unnamed_addr addrspace(1) constant [12 x i8] c"test failed\00" -@stats = external protected global %Tstats, align 1 +@stats2 = external protected global i16, align 1 ; CHECK-LABEL: main: -define protected noundef i32 @main(i32 %0, ptr nocapture readnone %1) addrspace(1) #0 { +define i32 @main() addrspace(1) { 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) + tail call addrspace(1) void @foo(i16 ptrtoint (ptr addrspace(1) @failed to i16), i16 11, i8 2, i16 32, ptr @stats2) %11 = load i64, ptr @ui1, align 8 %12 = load i64, ptr @ui2, align 8 @@ -30,5 +29,4 @@ entry: } 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" } +declare protected void @foo(i16, i16, i8, i16, i16) local_unnamed_addr addrspace(1) #0