[PATCH] Fix return sequence on armv4 thumb

[Jonathan Roelofs]

Tim,

Currently the return sequence we generate for thumb functions without varargs is 
something like this:

    pop {r7, pc}

which works on v5t and later processors. Unfortunately, this doesn't work on 
armv4t. According to 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204j/Cihfddaf.html, 
pop does not change the arm/thumb state on armv4. This means that we can't use 
it to return from functions that we expect to be interworking-safe.

 > POP, with reglist including the pc
 > This instruction causes a branch to the address popped off the stack into the
 > pc. This is usually a return from a subroutine, where the lr was pushed onto
 > the stack at the start of the subroutine.
 > In ARMv4, bits[1:0] of the address loaded must be 0b00. POP cannot be used to
 > change state.

 > The following restrictions apply to the 16-bit instructions:
 > For POP, reglist can only include the Lo registers and the pc

Therefore, the return sequence needs to be:
   pop { ... }
   pop {r3}
   bx r3

Attached is a patch that fixes this behavior.


Regards,
Jon


-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded
-------------- next part --------------
commit f55a03262695edbd045e6d8bef0abecddbd5f906
Author: Jonathan Roelofs <jonathan at codesourcery.com>
Date:   Thu Jul 31 14:11:18 2014 -0600

    [llvm] 'pop {pc}' does not set the arm/thumb bit on armv4, so use a different return sequence

diff --git a/llvm/lib/Target/ARM/Thumb1FrameLowering.cpp b/llvm/lib/Target/ARM/Thumb1FrameLowering.cpp
index baa97a7..a30dfdd 100644
--- a/llvm/lib/Target/ARM/Thumb1FrameLowering.cpp
+++ b/llvm/lib/Target/ARM/Thumb1FrameLowering.cpp
@@ -463,6 +463,7 @@ restoreCalleeSavedRegisters(MachineBasicBlock &MBB,
   MachineInstrBuilder MIB = BuildMI(MF, DL, TII.get(ARM::tPOP));
   AddDefaultPred(MIB);
 
+  bool IsReturn = false;
   bool NumRegs = false;
   for (unsigned i = CSI.size(); i != 0; --i) {
     unsigned Reg = CSI[i-1].getReg();
@@ -470,6 +471,9 @@ restoreCalleeSavedRegisters(MachineBasicBlock &MBB,
       // Special epilogue for vararg functions. See emitEpilogue
       if (isVarArg)
         continue;
+      IsReturn = true;
+      if (STI.hasV4TOps() && !STI.hasV5TOps())
+        continue;
       Reg = ARM::PC;
       (*MIB).setDesc(TII.get(ARM::tPOP_RET));
       MIB.copyImplicitOps(&*MI);
@@ -485,5 +489,27 @@ restoreCalleeSavedRegisters(MachineBasicBlock &MBB,
   else
     MF.DeleteMachineInstr(MIB);
 
+  // On armv4, popping into PC will not change arm/thumb state,
+  // so instead we have to emit that as:
+  //   POP {r3}
+  //   BX r3
+  if (IsReturn && STI.hasV4TOps() && !STI.hasV5TOps()) {
+    // Get the last instruction, tBX_RET
+    MI = MBB.getLastNonDebugInstr();
+    assert (MI->getOpcode() == ARM::tBX_RET);
+    DebugLoc dl = MI->getDebugLoc();
+    // Epilogue: pop LR to R3 and branch off it.
+    AddDefaultPred(BuildMI(MBB, MI, dl, TII.get(ARM::tPOP)))
+      .addReg(ARM::R3, RegState::Define);
+
+    MachineInstrBuilder MIB =
+      BuildMI(MBB, MI, dl, TII.get(ARM::tBX))
+      .addReg(ARM::R3, RegState::Kill);
+    AddDefaultPred(MIB);
+    MIB.copyImplicitOps(&*MI);
+    // erase the old tBX_RET instruction
+    MBB.erase(MI);
+  }
+
   return true;
 }