Use R0/R1 instead of R4/R5 for ARM eh_return return values

Returning things in callee-save registers can clobber live variables in
functions with catch blocks. I'm not 100% sure how this ever worked.

The side effect of this means I had to fix the return behavior of functions
which return via eh_return so that they save and restore R0/R1 in addition
to the callee-save registers. This is all part of our ARM dwarf EH localmod,
but doing this same thing on x86 should fix PR8541.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4171
[email protected]

Review URL: https://codereview.chromium.org/1154253004

Author: dschuff

lib/Target/ARM/ARMBaseRegisterInfo.cpp

@@ -24,6 +24,7 @@
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
+#include "llvm/CodeGen/MachineModuleInfo.h" // @LOCALMOD
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/RegisterScavenging.h"
 #include "llvm/CodeGen/VirtRegMap.h"
@@ -63,7 +64,14 @@ static unsigned getFramePointerReg(const ARMSubtarget &STI) {
 const MCPhysReg*
 ARMBaseRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
   const ARMSubtarget &STI = MF->getSubtarget<ARMSubtarget>();
-  if (STI.isTargetNaCl()) return CSR_NaCl_SaveList; // @LOCALMOD
+  // @LOCALMOD-START
+  // We don't support special calling conventions for NaCl here.
+  if (STI.isTargetNaCl()) {
+    if (MF->getMMI().callsEHReturn())
+      return CSR_NaCl_EHRet_SaveList;
+    return CSR_NaCl_SaveList;
+  }
+  // @LOCALMOD-END
 
   const MCPhysReg *RegList =
       STI.isTargetDarwin() ? CSR_iOS_SaveList : CSR_AAPCS_SaveList;

lib/Target/ARM/ARMCallingConv.td

@@ -242,4 +242,5 @@ def CSR_FIQ : CalleeSavedRegs<(add LR, R11, (sequence "R%u", 7, 0))>;
 // NaCl does not save R9, but otherwise uses the same order as AAPCS
 def CSR_NaCl : CalleeSavedRegs<(add LR, R11, R10, R8, R7, R6, R5, R4,
                                      (sequence "D%u", 15, 8))>;
+def CSR_NaCl_EHRet : CalleeSavedRegs<(add CSR_NaCl, R1, R0)>;
 // @LOCALMOD-END

lib/Target/ARM/ARMFrameLowering.cpp

@@ -966,6 +966,15 @@ void ARMFrameLowering::emitPushInst(MachineBasicBlock &MBB,
           isKill = false;
       }
 
+      // @LOCALMOD-START
+      // Functions which call EHReturn spill all CSRs plus R0/R1
+      if ((Reg == ARM::R0 || Reg == ARM::R1) &&
+          MF.getRegInfo().isLiveIn(Reg)) {
+        assert(MF.getMMI().callsEHReturn());
+        isKill = false;
+      }
+      // @LOCALMOD-END
+
       if (isKill)
         MBB.addLiveIn(Reg);
 
@@ -1025,6 +1034,7 @@ void ARMFrameLowering::emitPopInst(MachineBasicBlock &MBB,
   while (i != 0) {
     unsigned LastReg = 0;
     bool DeleteRet = false;
+    unsigned SkippedPop = 0; // @LOCALMOD
     for (; i != 0; --i) {
       unsigned Reg = CSI[i-1].getReg();
       if (!(Func)(Reg, STI.isTargetDarwin())) continue;
@@ -1033,6 +1043,19 @@ void ARMFrameLowering::emitPopInst(MachineBasicBlock &MBB,
       if (Reg >= ARM::D8 && Reg < ARM::D8 + NumAlignedDPRCS2Regs)
         continue;
 
+      // @LOCALMOD-START
+      // Functions which call EHReturn spill all of their CSRs plus R0 and R1,
+      // (the EHReturn return value registers). Epilogs which return via
+      // EHreturn pop all of them, but epilogs which return via normal return
+      // must not restore R0 and R1, as that would clobber the return value.
+      if (MF.getMMI().callsEHReturn() &&
+          RetOpcode != ARM::ARMeh_return &&
+          (Reg == ARM::R0 || Reg == ARM::R1)) {
+        SkippedPop++;
+        continue;
+      }
+      // @LOCALMOD-END
+
       if (Reg == ARM::LR && !isTailCall && !isVarArg && !isInterrupt &&
           STI.hasV5TOps() &&
           !STI.isTargetNaCl() /* @LOCALMOD */) {
@@ -1052,6 +1075,21 @@ void ARMFrameLowering::emitPopInst(MachineBasicBlock &MBB,
       Regs.push_back(Reg);
     }
 
+    // @LOCALMOD-START
+    if (SkippedPop) {
+      // We need to increment the stack pointer to compensate for the skipped
+      // pops. However we cannot directly increment it because the epilog
+      // insertion code places the stack pointer restore before the CSR
+      // restores; it does this by finding the first instruction that's not a
+      // pop.  If we put an add here, the restore would go in between the
+      // restore of the FP registers and the GPRs, instead of before the FP
+      // restore. So use a pop into R12 to adjust SP.
+      while(SkippedPop--)
+         AddDefaultPred(BuildMI(MBB, MI, DL, TII.get(LdmOpc), ARM::SP)
+                         .addReg(ARM::SP)).addReg(ARM::R12);
+    }
+    // @LOCALMOD-END
+
     if (Regs.empty())
       continue;
     if (Regs.size() > 1 || LdrOpc == 0) {

lib/Target/ARM/ARMISelLowering.cpp

@@ -765,23 +765,16 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
   if (!Subtarget->isTargetMachO()) {
     // Non-MachO platforms may return values in these registers via the
     // personality function.
-    // @LOCALMOD-START
-    if (Subtarget->isTargetNaCl()) {
-      // we use the first caller saved regs here
-      // c.f.: llvm-gcc/llvm-gcc-4.2/gcc/unwind-dw2.c::uw_install_context
-      // NOTE: these are related to the _Unwind_PNaClSetResult{0,1} functions
-      setExceptionPointerRegister(ARM::R4);
-      setExceptionSelectorRegister(ARM::R5);
-  
-      setOperationAction(ISD::FRAME_TO_ARGS_OFFSET, MVT::i32, Custom);
+    setExceptionPointerRegister(ARM::R0);
+    setExceptionSelectorRegister(ARM::R1);
+  }
 
-      setOperationAction(ISD::EH_RETURN, MVT::Other, Custom);
-    } else {
-      setExceptionPointerRegister(ARM::R0);
-      setExceptionSelectorRegister(ARM::R1);
-    }
-    // @LOCALMOD-END
+  // @LOCALMOD-START
+  if (Subtarget->isTargetNaCl()) {
+    setOperationAction(ISD::FRAME_TO_ARGS_OFFSET, MVT::i32, Custom);
+    setOperationAction(ISD::EH_RETURN, MVT::Other, Custom);
   }
+  // @LOCALMOD-END
 
   if (Subtarget->getTargetTriple().isWindowsItaniumEnvironment())
     setOperationAction(ISD::DYNAMIC_STACKALLOC, MVT::i32, Custom);

test/CodeGen/ARM/ehreturn.ll

@@ -0,0 +1,25 @@
+; RUN: llc -O0 < %s | FileCheck %s
+target datalayout = "e-p:32:32-i64:64-n32"
+target triple = "armv7-unknown-nacl-gnueabihf"
+
+declare void @llvm.eh.return.i32(i32, i8*) nounwind
+declare void @llvm.eh.unwind.init() nounwind
+
+; CHECK-LABEL: @raise_exception
+; CHECK: push {r0, r1, r4, r5, r6, r7, r8, r10, r11, lr}
+define i32 @raise_exception(i32 %b) {
+entry:
+  tail call void @llvm.eh.unwind.init()
+  %a = add i32 %b, 3
+  %cmp = icmp slt i32 %a, 0
+  br i1 %cmp, label %normal_ret, label %eh_ret
+normal_ret:
+; CHECK: ldm sp!, {r12}
+; CHECK: ldm sp!, {r12}
+; CHECK: pop {r4, r5, r6, r7, r8, r10, r11, lr}
+  ret i32 %a
+eh_ret:
+; CHECK: pop {r0, r1, r4, r5, r6, r7, r8, r10, r11, lr}
+  tail call void @llvm.eh.return.i32(i32 %a, i8* null)
+  unreachable
+}