[llvm] r301887 - [AVR] Fix a bug where the frame pointer is clobbered

[Dylan McKay via llvm-commits]

Author: dylanmckay
Date: Mon May  1 19:11:34 2017
New Revision: 301887

URL: http://llvm.org/viewvc/llvm-project?rev=301887&view=rev
Log:
[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.

Modified:
    llvm/trunk/lib/Target/AVR/AVRFrameLowering.cpp

Modified: llvm/trunk/lib/Target/AVR/AVRFrameLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AVR/AVRFrameLowering.cpp?rev=301887&r1=301886&r2=301887&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AVR/AVRFrameLowering.cpp (original)
+++ llvm/trunk/lib/Target/AVR/AVRFrameLowering.cpp Mon May  1 19:11:34 2017
@@ -57,6 +57,7 @@ void AVRFrameLowering::emitPrologue(Mach
   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(Mach
   // 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(Mach
   }
 
   // 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(Mach
         .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::determineCalleeSa
                                             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.
 ///