From b0cacfc25f613529b8aaba24b6db7baef589612d Mon Sep 17 00:00:00 2001
From: Dylan McKay <me@dylanmckay.io>
Date: Tue, 2 May 2017 00:11:34 +0000
Subject: [PATCH] [AVR] Fix a bug where the frame pointer is clobbered

Because it was a callee-saved register, we automatically generated code
to spill and unspill its original value so that it is restored after the
function returns.

The problem is that this code was being generated before the epilogue.
The epilogue itself uses the Y register, which could be prematurely
restored by the CSR restoration process.

This removes R29R28 from the CSR list and changes the prologue/epilogue
code to handle it explicitly.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@301887 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Target/AVR/AVRFrameLowering.cpp | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/lib/Target/AVR/AVRFrameLowering.cpp b/lib/Target/AVR/AVRFrameLowering.cpp
index 7e0e76182fa..3c37a1a9989 100644
--- a/lib/Target/AVR/AVRFrameLowering.cpp
+++ b/lib/Target/AVR/AVRFrameLowering.cpp
@@ -57,6 +57,7 @@ void AVRFrameLowering::emitPrologue(MachineFunction &MF,
   DebugLoc DL = (MBBI != MBB.end()) ? MBBI->getDebugLoc() : DebugLoc();
   const AVRSubtarget &STI = MF.getSubtarget<AVRSubtarget>();
   const AVRInstrInfo &TII = *STI.getInstrInfo();
+  bool HasFP = hasFP(MF);
 
   // Interrupt handlers re-enable interrupts in function entry.
   if (CallConv == CallingConv::AVR_INTR) {
@@ -69,9 +70,18 @@ void AVRFrameLowering::emitPrologue(MachineFunction &MF,
   // handlers before saving any other registers.
   if (CallConv == CallingConv::AVR_INTR ||
       CallConv == CallingConv::AVR_SIGNAL) {
+
+    // 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);
+    }
+
     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)
         .addImm(0x3f)
         .setMIFlag(MachineInstr::FrameSetup);
@@ -86,7 +96,7 @@ void AVRFrameLowering::emitPrologue(MachineFunction &MF,
   }
 
   // Early exit if the frame pointer is not needed in this function.
-  if (!hasFP(MF)) {
+  if (!HasFP) {
     return;
   }
 
@@ -163,6 +173,9 @@ void AVRFrameLowering::emitEpilogue(MachineFunction &MF,
         .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.
@@ -408,12 +421,9 @@ void AVRFrameLowering::determineCalleeSaves(MachineFunction &MF,
                                             RegScavenger *RS) const {
   TargetFrameLowering::determineCalleeSaves(MF, SavedRegs, RS);
 
-  // Spill register Y when it is used as the frame pointer.
-  if (hasFP(MF)) {
-    SavedRegs.set(AVR::R29R28);
-    SavedRegs.set(AVR::R29);
-    SavedRegs.set(AVR::R28);
-  }
+  // 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.
 }
 /// The frame analyzer pass.
 ///
-- 
2.40.0