[ARM][Thumb] Save FPSCR + FPEXC for save-vfp attribute

Previously, the ARM frame code had 5 "phases" of saving registers.

- GPRCS1
- GPRCS2 (splitFramePushPop)
- DPRCS
- GPRCS2 (splitFramePointerPush)
- DPRCS2

There are 2 GPRCS2's because the frame code is assuming those two are mutually
exclusive, and is therefore reusing the name for the zone on the stack. After
some renaming and creating an extra space for saving the FP status registers,
we now have:

- GPRCS1
- GPRCS2 (splitFramePushPop)
- FPStatusRegs (new)
- DPRCS1
- GPRCS3 (splitFramePointerPush)
- DPRCS2

FPSCR and FPEXC will be stored in FPStatusRegs, after GPRCS1 has been
saved (and GPRCS2, if applicable). FPSCR is present on all targets with a VFP,
but the FPEXC register is not present on Cortex-M devices, so different amounts
of bytes are being pushed onto the stack depending on our target, which would
affect alignment for subsequent saves.

Thankfully, DPRCS1 will sum up all previous bytes that were saved, and will
emit extra instructions to ensure that its alignment is correct. My assumption
is that if DPRCS1 is able to correct its alignment to be correct, then all
subsequent saves will also have correct alignment.

Author: pestctrl

clang/test/CodeGen/arm-interrupt-save-fp-attr-status-regs.c

@@ -0,0 +1,34 @@
+// REQUIRES: arm-registered-target
+// RUN: %clang -target arm-none-none-eabihf -mcpu=cortex-r5 -mfpu=vfpv3-d16 -marm -S -o - %s \
+// RUN: | FileCheck %s --check-prefix=CHECK-R
+// RUN: %clang -target arm-none-none-eabihf -mcpu=cortex-r5 -mfpu=vfpv3-d16 -mthumb -S -o - %s \
+// RUN: | FileCheck %s --check-prefix=CHECK-R
+// RUN: %clang -target arm-none-none-eabihf -mcpu=cortex-r4 -mfpu=vfpv3-d16 -marm -S -o - %s \
+// RUN: | FileCheck %s --check-prefix=CHECK-R
+// RUN: %clang -target arm-none-none-eabihf -mcpu=cortex-r4 -mfpu=vfpv3-d16 -mthumb -S -o - %s \
+// RUN: | FileCheck %s --check-prefix=CHECK-R
+// RUN: %clang -target arm-none-none-eabihf -mcpu=cortex-m4 -mfpu=fpv4-sp-d16 -S -o - %s \
+// RUN: | FileCheck %s --check-prefix=CHECK-M
+// RUN: %clang -target arm-none-none-eabihf -mcpu=cortex-m33 -mfpu=fpv5-sp-d16 -S -o - %s \
+// RUN: | FileCheck %s --check-prefix=CHECK-M
+
+void bar();
+
+__attribute__((interrupt_save_fp)) void test_generic_interrupt() {
+    // CHECK-R:      vmrs	r4, fpscr
+    // CHECK-R-NEXT: vmrs	r5, fpexc
+    // CHECK-R-NEXT: .save  {fpscr, fpexc}
+    // CHECK-R-NEXT: push	{r4, r5}
+    // .....
+    // CHECK-R:      pop	{r4, r5}
+    // CHECK-R-NEXT: vmsr	fpscr, r4
+    // CHECK-R-NEXT: vmsr	fpexc, r5
+
+    // CHECK-M:      vmrs	r4, fpscr
+    // CHECK-M-NEXT: .save  {fpscr}
+    // CHECK-M-NEXT: push	{r4}
+    // .....
+    // CHECK-M:      pop	{r4}
+    // CHECK-M-NEXT: vmsr	fpscr, r4
+    bar();
+}

llvm/include/llvm/IR/IntrinsicsARM.td

@@ -311,7 +311,7 @@ def int_arm_isb : ClangBuiltin<"__builtin_arm_isb">, MSBuiltin<"__isb">,
 // VFP
 
 def int_arm_get_fpscr : ClangBuiltin<"__builtin_arm_get_fpscr">,
-                       DefaultAttrsIntrinsic<[llvm_i32_ty], [], []>;
+                       DefaultAttrsIntrinsic<[llvm_i32_ty], [], [IntrReadMem]>;
 def int_arm_set_fpscr : ClangBuiltin<"__builtin_arm_set_fpscr">,
                        DefaultAttrsIntrinsic<[], [llvm_i32_ty], []>;
 def int_arm_vcvtr : DefaultAttrsIntrinsic<[llvm_float_ty],

llvm/lib/Target/ARM/ARMAsmPrinter.cpp

@@ -1207,6 +1207,14 @@ void ARMAsmPrinter::EmitUnwindingInstruction(const MachineInstr *MI) {
     SrcReg = ~0U;
     DstReg = MI->getOperand(0).getReg();
     break;
+  case ARM::VMRS:
+    SrcReg = ARM::FPSCR;
+    DstReg = MI->getOperand(0).getReg();
+    break;
+  case ARM::VMRS_FPEXC:
+    SrcReg = ARM::FPEXC;
+    DstReg = MI->getOperand(0).getReg();
+    break;
   default:
     SrcReg = MI->getOperand(1).getReg();
     DstReg = MI->getOperand(0).getReg();
@@ -1369,6 +1377,13 @@ void ARMAsmPrinter::EmitUnwindingInstruction(const MachineInstr *MI) {
         // correct ".save" later.
         AFI->EHPrologueRemappedRegs[DstReg] = SrcReg;
         break;
+      case ARM::VMRS:
+      case ARM::VMRS_FPEXC:
+        // If a function spills FPSCR or FPEXC, we copy the values to low
+        // registers before pushing them. Record the copy so we can emit the
+        // correct ".save" later.
+        AFI->EHPrologueRemappedRegs[DstReg] = SrcReg;
+        break;
       case ARM::tLDRpci: {
         // Grab the constpool index and check, whether it corresponds to
         // original or cloned constpool entry.

llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp

@@ -79,42 +79,42 @@ ARMBaseRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
                : (UseSplitPush ? CSR_ATPCS_SplitPush_SwiftTail_SaveList
                                : CSR_AAPCS_SwiftTail_SaveList);
   } else if (F.hasFnAttribute("interrupt")) {
-    bool SaveFP = F.hasFnAttribute("save-fp");
-    bool HasNEON = MF->getSubtarget<ARMSubtarget>().hasNEON();
+
+    // Don't bother saving the floating point registers if target is not hard
+    // float. This will prevent the Thumb1FrameLowering (cortex-m0) from
+    // crashing due to an llvm_unreachable being triggered when a D-class
+    // register is in the calling convention.
+    if (STI.isTargetHardFloat() && F.hasFnAttribute("save-fp")) {
+      bool HasNEON = STI.hasNEON();
+
+      if (STI.isMClass()) {
+        assert(!HasNEON && "NEON is only for Cortex-R/A");
+        return UseSplitPush ? CSR_ATPCS_SplitPush_FP_SaveList
+                            : CSR_AAPCS_FP_SaveList;
+      }
+      if (F.getFnAttribute("interrupt").getValueAsString() == "FIQ") {
+        return HasNEON ? CSR_FIQ_FP_NEON_SaveList : CSR_FIQ_FP_SaveList;
+      }
+      return HasNEON ? CSR_GenericInt_FP_NEON_SaveList
+                     : CSR_GenericInt_FP_SaveList;
+    }
 
     if (STI.isMClass()) {
       // M-class CPUs have hardware which saves the registers needed to allow a
       // function conforming to the AAPCS to function as a handler.
       // Additionally, M Class has hardware support for saving VFP registers,
       // but the option can be disabled
-      if (SaveFP) {
-        if (HasNEON) {
-          return UseSplitPush ? CSR_AAPCS_SplitPush_FP_NEON_SaveList
-                              : CSR_AAPCS_FP_NEON_SaveList;
-        } else {
-          return UseSplitPush ? CSR_AAPCS_SplitPush_FP_SaveList
-                              : CSR_AAPCS_FP_SaveList;
-        }
-      } else {
-        return UseSplitPush ? CSR_AAPCS_SplitPush_SaveList : CSR_AAPCS_SaveList;
-      }
-    } else if (F.getFnAttribute("interrupt").getValueAsString() == "FIQ") {
+      return UseSplitPush ? CSR_ATPCS_SplitPush_SaveList : CSR_AAPCS_SaveList;
+    }
+
+    if (F.getFnAttribute("interrupt").getValueAsString() == "FIQ") {
       // Fast interrupt mode gives the handler a private copy of R8-R14, so less
       // need to be saved to restore user-mode state.
-      if (SaveFP) {
-        return HasNEON ? CSR_FIQ_FP_NEON_SaveList : CSR_FIQ_FP_SaveList;
-      } else {
-        return CSR_FIQ_SaveList;
-      }
+      return CSR_FIQ_SaveList;
     } else {
       // Generally only R13-R14 (i.e. SP, LR) are automatically preserved by
       // exception handling.
-      if (SaveFP) {
-        return HasNEON ? CSR_GenericInt_FP_NEON_SaveList
-                       : CSR_GenericInt_FP_SaveList;
-      } else {
-        return CSR_GenericInt_SaveList;
-      }
+      return CSR_GenericInt_SaveList;
     }
   }
 

llvm/lib/Target/ARM/ARMCallingConv.td

@@ -267,7 +267,7 @@ def CC_ARM_Win32_CFGuard_Check : CallingConv<[
 def CSR_NoRegs : CalleeSavedRegs<(add)>;
 def CSR_FPRegs : CalleeSavedRegs<(add (sequence "D%u", 0, 31))>;
 
-def CSR_FP_Interrupt_Regs : CalleeSavedRegs<(add (sequence "D%u", 7, 0))>;
+def CSR_FP_Interrupt_Regs : CalleeSavedRegs<(add FPSCR, FPEXC, (sequence "D%u", 15, 0))>;
 def CSR_FP_NEON_Interrupt_Regs : CalleeSavedRegs<(add CSR_FP_Interrupt_Regs, 
                                                   (sequence "D%u", 31, 16))>;
 
@@ -276,20 +276,6 @@ def CSR_AAPCS : CalleeSavedRegs<(add LR, R11, R10, R9, R8, R7, R6, R5, R4,
 
 def CSR_AAPCS_FP : CalleeSavedRegs<(add CSR_AAPCS, CSR_FP_Interrupt_Regs)>;
 
-def CSR_AAPCS_FP_NEON : CalleeSavedRegs<(add CSR_AAPCS_FP, 
-                                         CSR_FP_NEON_Interrupt_Regs)>;
-
-// The Windows Control Flow Guard Check function preserves the same registers as
-// AAPCS, and also preserves all floating point registers.
-def CSR_Win_AAPCS_CFGuard_Check : CalleeSavedRegs<(add LR, R11, R10, R9, R8, R7,
-                                     R6, R5, R4, (sequence "D%u", 15, 0))>;
-
-// R8 is used to pass swifterror, remove it from CSR.
-def CSR_AAPCS_SwiftError : CalleeSavedRegs<(sub CSR_AAPCS, R8)>;
-
-// R10 is used to pass swiftself, remove it from CSR.
-def CSR_AAPCS_SwiftTail : CalleeSavedRegs<(sub CSR_AAPCS, R10)>;
-
 // The order of callee-saved registers needs to match the order we actually push
 // them in FrameLowering, because this order is what's used by
 // PrologEpilogInserter to allocate frame index slots. So when R7 is the frame
@@ -302,11 +288,19 @@ def CSR_Win_SplitFP : CalleeSavedRegs<(add R10, R9, R8, R7, R6, R5, R4,
                                                (sequence "D%u", 15, 8),
                                                LR, R11)>;
 
-def CSR_AAPCS_SplitPush_FP : CalleeSavedRegs<(add CSR_AAPCS_SplitPush, 
+def CSR_ATPCS_SplitPush_FP : CalleeSavedRegs<(add CSR_ATPCS_SplitPush,
                                               CSR_FP_Interrupt_Regs)>;
 
-def CSR_AAPCS_SplitPush_FP_NEON : CalleeSavedRegs<(add CSR_AAPCS_SplitPush_FP, 
-                                                   CSR_FP_NEON_Interrupt_Regs)>;
+// The Windows Control Flow Guard Check function preserves the same registers as
+// AAPCS, and also preserves all floating point registers.
+def CSR_Win_AAPCS_CFGuard_Check : CalleeSavedRegs<(add LR, R11, R10, R9, R8, R7,
+                                     R6, R5, R4, (sequence "D%u", 15, 0))>;
+
+// R8 is used to pass swifterror, remove it from CSR.
+def CSR_AAPCS_SwiftError : CalleeSavedRegs<(sub CSR_AAPCS, R8)>;
+
+// R10 is used to pass swiftself, remove it from CSR.
+def CSR_AAPCS_SwiftTail : CalleeSavedRegs<(sub CSR_AAPCS, R10)>;
 
 
 // R8 is used to pass swifterror, remove it from CSR.