[AVR] Fix stack size in functions with a frame pointer

[aykevl]

Summary

This patch fixes a bug in stack save/restore code. Because the frame pointer was saved/restored manually (not by marking it as clobbered) the StackSize variable was not updated accordingly. Most code still worked, but code that tried to load a parameter passed on the stack did not.

This commit fixes this by marking the frame pointer as a callee-clobbered register. This will let it be saved without any effort in prolog/epilog code and will make sure the correct address is calculated for loading parameters that are passed on the stack.

This approach is used by most other targets (such as X86, AArch64 and RISC-V).

---

Note that in two of the tests (calling-conv/c/stack.ll and return.ll) the updated assembly looks a lot better, I think the code was buggy before. I find it hard to say that varargs.ll has been fixed, I'm not exactly sure what the code is doing. That said, this patch does get mulodi4_test.c (in compiler-rt) to pass so it definitely fixes a bug.

[dylanmckay]

Added an integration test case

https://github.com/dylanmckay/avr-compiler-integration-tests/commit/cc5b20f9045561ffa7e076d394b2594a12b90d87

As always, great work @aykevl

llvm/lib/Target/AVR/AVRFrameLowering.cpp

void AVRFrameLowering::emitPrologue(MachineFunction &MF,
 
   // Interrupt handlers re-enable interrupts in function entry.
   if (AFI->isInterruptHandler()) {
     BuildMI(MBB, MBBI, DL, TII.get(AVR::BSETs))
         .addImm(0x07)
         .setMIFlag(MachineInstr::FrameSetup);
   }
 
-  // Save the frame pointer if we have one.
-  if (HasFP) {
-    BuildMI(MBB, MBBI, DL, TII.get(AVR::PUSHWRr))
-        .addReg(AVR::R29R28, RegState::Kill)
-        .setMIFlag(MachineInstr::FrameSetup);
-  }
-
   // Emit special prologue code to save R1, R0 and SREG in interrupt/signal
   // handlers before saving any other registers.
   if (AFI->isInterruptOrSignalHandler()) {
     BuildMI(MBB, MBBI, DL, TII.get(AVR::PUSHWRr))
         .addReg(AVR::R1R0, RegState::Kill)
         .setMIFlag(MachineInstr::FrameSetup);
 
     BuildMI(MBB, MBBI, DL, TII.get(AVR::INRdA), AVR::R0)
void AVRFrameLowering::emitEpilogue(MachineFunction &MF,
   if (AFI->isInterruptOrSignalHandler()) {
     BuildMI(MBB, MBBI, DL, TII.get(AVR::POPRd), AVR::R0);
     BuildMI(MBB, MBBI, DL, TII.get(AVR::OUTARr))
         .addImm(0x3f)
         .addReg(AVR::R0, RegState::Kill);
     BuildMI(MBB, MBBI, DL, TII.get(AVR::POPWRd), AVR::R1R0);
   }
 
-  if (hasFP(MF))
-    BuildMI(MBB, MBBI, DL, TII.get(AVR::POPWRd), AVR::R29R28);
-
   // Early exit if there is no need to restore the frame pointer.
   if (!FrameSize) {
     return;
   }
 
   // Skip the callee-saved pop instructions.
   while (MBBI != MBB.begin()) {
     MachineBasicBlock::iterator PI = std::prev(MBBI);

 }
 
 void AVRFrameLowering::determineCalleeSaves(MachineFunction &MF,
                                             BitVector &SavedRegs,
                                             RegScavenger *RS) const {
   TargetFrameLowering::determineCalleeSaves(MF, SavedRegs, RS);
 
   // If we have a frame pointer, the Y register needs to be saved as well.
-  // We don't do that here however - the prologue and epilogue generation
-  // code will handle it specially.
+  if (hasFP(MF)) {
+    SavedRegs.set(AVR::R29);
+    SavedRegs.set(AVR::R28);
+  }
 }
 /// The frame analyzer pass.
 ///
 /// Scans the function for allocas and used arguments
 /// that are passed through the stack.
 struct AVRFrameAnalyzer : public MachineFunctionPass {
   static char ID;
   AVRFrameAnalyzer() : MachineFunctionPass(ID) {}

llvm/test/CodeGen/AVR/calling-conv/c/stack.ll

 ; RUN: llc < %s -march=avr | FileCheck %s
 
 ; CHECK-LABEL: ret_void_args_i64_i64_i32
 define void @ret_void_args_i64_i64_i32(i64 %a, i64 %b, i32 %c) {
   ; We're goign to clobber PTRREG Y
   ; CHECK:      push    r28
   ; CHECK-NEXT: push    r29
 
   ; Load the stack pointer into Y.
   ; CHECK-NEXT: in      r28, 61
   ; CHECK-NEXT: in      r29, 62
 
   ; Load the top two bytes from the 32-bit int.
-  ; CHECK-NEXT: ldd     r24, Y+5
-  ; CHECK-NEXT: ldd     r25, Y+6
+  ; CHECK-NEXT: ldd     r24, Y+7
+  ; CHECK-NEXT: ldd     r25, Y+8
   ; Store the top two bytes of the 32-bit int to memory.
   ; CHECK-NEXT: sts     7, r25
   ; CHECK-NEXT: sts     6, r24
 
   ; Load the bottom two bytes from the 32-bit int.
-  ; CHECK-NEXT: ldd     r24, Y+3
-  ; CHECK-NEXT: ldd     r25, Y+4
+  ; CHECK-NEXT: ldd     r24, Y+5
+  ; CHECK-NEXT: ldd     r25, Y+6
   ; Store the bottom two bytes of the 32-bit int to memory.
   ; CHECK-NEXT: sts     5, r25
   ; CHECK-NEXT: sts     4, r24
 
   ; Restore PTRREG Y
   ; CHECK-NEXT: pop     r29
   ; CHECK-NEXT: pop     r28
   store volatile i32 %c, i32* inttoptr (i64 4 to i32*)
   ret void
 }

llvm/test/CodeGen/AVR/return.ll

 ; CHECK-NEXT: ret
     ret i64 %x
 }
 
 define i64 @return64_arg2(i64 %x, i64 %y, i64 %z) {
 ; CHECK-LABEL: return64_arg2:
 ; CHECK: push r28
 ; CHECK: push r29
-; CHECK: ldd r18, Y+3
-; CHECK: ldd r19, Y+4
-; CHECK: ldd r20, Y+5
-; CHECK: ldd r21, Y+6
-; CHECK: ldd r22, Y+7
-; CHECK: ldd r23, Y+8
-; CHECK: ldd r24, Y+9
-; CHECK: ldd r25, Y+10
+; CHECK: ldd r18, Y+5
+; CHECK: ldd r19, Y+6
+; CHECK: ldd r20, Y+7
+; CHECK: ldd r21, Y+8
+; CHECK: ldd r22, Y+9
+; CHECK: ldd r23, Y+10
+; CHECK: ldd r24, Y+11
+; CHECK: ldd r25, Y+12
 ; CHECK: pop r29
 ; CHECK: pop r28
     ret i64 %z
 }
 
 define i32 @return64_trunc(i32 %a, i32 %b, i32 %c, i64 %d) {
 ; CHECK-LABEL: return64_trunc:
 ; CHECK: push r28
 ; CHECK: push r29
-; CHECK: ldd r22, Y+3
-; CHECK: ldd r23, Y+4
-; CHECK: ldd r24, Y+5
-; CHECK: ldd r25, Y+6
+; CHECK: ldd r22, Y+5
+; CHECK: ldd r23, Y+6
+; CHECK: ldd r24, Y+7
+; CHECK: ldd r25, Y+8
 ; CHECK: pop r29
 ; CHECK: pop r28
   %result = trunc i64 %d to i32
   ret i32 %result
 }
 
 define avr_intrcc void @interrupt_handler() {
 ; CHECK-LABEL: interrupt_handler:

llvm/test/CodeGen/AVR/varargs.ll

 ; RUN: llc -mattr=sram,movw,addsubiw < %s -march=avr | FileCheck %s
 
 declare void @llvm.va_start(i8*)
 declare i16 @vsprintf(i8* nocapture, i8* nocapture, i8*)
 declare void @llvm.va_end(i8*)
 
 define i16 @varargs1(i8* nocapture %x, ...) {
 ; CHECK-LABEL: varargs1:
 ; CHECK: movw r20, r28
-; CHECK: subi r20, 217
+; CHECK: subi r20, 215
 ; CHECK: sbci r21, 255
 ; CHECK: movw r24, r28
 ; CHECK: adiw r24, 3
-; CHECK: ldd r22, Y+37
-; CHECK: ldd r23, Y+38
+; CHECK: ldd r22, Y+39
+; CHECK: ldd r23, Y+40
 ; CHECK: call
   %buffer = alloca [32 x i8]
   %ap = alloca i8*
   %ap1 = bitcast i8** %ap to i8*
   call void @llvm.va_start(i8* %ap1)
   %arraydecay = getelementptr inbounds [32 x i8], [32 x i8]* %buffer, i16 0, i16 0
   %1 = load i8*, i8** %ap
   %call = call i16 @vsprintf(i8* %arraydecay, i8* %x, i8* %1)