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

This reverts commit 1f05703.

Author: pestctrl

clang/include/clang/Basic/Attr.td

@@ -994,22 +994,6 @@ def ARMInterrupt : InheritableAttr, TargetSpecificAttr<TargetARM> {
   let Documentation = [ARMInterruptDocs];
 }
 
-def ARMInterruptSaveFP : InheritableAttr, TargetSpecificAttr<TargetARM> {
-  let Spellings = [GNU<"interrupt_save_fp">];
-  let Args = [EnumArgument<"Interrupt", "InterruptType", /*is_string=*/true,
-                           ["IRQ", "FIQ", "SWI", "ABORT", "UNDEF", ""],
-                           ["IRQ", "FIQ", "SWI", "ABORT", "UNDEF", "Generic"],
-                           1>];
-  let HasCustomParsing = 0;
-  let Documentation = [ARMInterruptSaveFPDocs];
-}
-
-def ARMSaveFP : InheritableAttr, TargetSpecificAttr<TargetARM> {
-  let Spellings = [];
-  let Subjects = SubjectList<[Function]>;
-  let Documentation = [InternalOnly];
-}
-
 def AVRInterrupt : InheritableAttr, TargetSpecificAttr<TargetAVR> {
   let Spellings = [GCC<"interrupt">];
   let Subjects = SubjectList<[Function]>;

clang/include/clang/Basic/AttrDocs.td

@@ -2662,19 +2662,6 @@ The semantics are as follows:
   }];
 }
 
-def ARMInterruptSaveFPDocs : Documentation {
-    let Category = DocCatFunction;
-  let Heading = "interrupt_save_fp (ARM)";
-  let Content = [{
-Clang supports the GNU style ``__attribute__((interrupt_save_fp("TYPE")))``
-on ARM targets. This attribute behaves the same way as the ARM interrupt
-attribute, except the general purpose floating point registers are also saved,
-along with FPEXC and FPSCR. Note, even on M-class CPUs, where the floating
-point context can be automatically saved depending on the FPCCR, the general
-purpose floating point registers will be saved.
-  }];
-}
-
 def BPFPreserveAccessIndexDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -346,14 +346,8 @@ def warn_anyx86_excessive_regsave : Warning<
   InGroup<DiagGroup<"excessive-regsave">>;
 def warn_arm_interrupt_vfp_clobber : Warning<
   "interrupt service routine with vfp enabled may clobber the "
-  "interruptee's vfp state; "
-  "consider using the `interrupt_save_fp` attribute to prevent this behavior">,
+  "interruptee's vfp state">,
   InGroup<DiagGroup<"arm-interrupt-vfp-clobber">>;
-def warn_arm_interrupt_save_fp_without_vfp_unit : Warning<
-   "`interrupt_save_fp` only applies to targets that have a VFP unit enabled "
-   "for this compilation; this will be treated as a regular `interrupt` "
-   "attribute">,
-   InGroup<DiagGroup<"arm-interrupt-vfp-clobber">>;
 def err_arm_interrupt_called : Error<
   "interrupt service routine cannot be called directly">;
 def warn_interrupt_signal_attribute_invalid : Warning<

clang/lib/CodeGen/Targets/ARM.cpp

@@ -190,12 +190,6 @@ class ARMTargetCodeGenInfo : public TargetCodeGenInfo {
 
     Fn->addFnAttr("interrupt", Kind);
 
-    // Note: the ARMSaveFPAttr can only exist if we also have an interrupt
-    // attribute
-    const ARMSaveFPAttr *SaveFPAttr = FD->getAttr<ARMSaveFPAttr>();
-    if (SaveFPAttr)
-      Fn->addFnAttr("save-fp");
-
     ARMABIKind ABI = getABIInfo<ARMABIInfo>().getABIKind();
     if (ABI == ARMABIKind::APCS)
       return;

clang/lib/Sema/SemaARM.cpp

@@ -1307,11 +1307,9 @@ void SemaARM::handleInterruptAttr(Decl *D, const ParsedAttr &AL) {
     return;
   }
 
-  if (!D->hasAttr<ARMSaveFPAttr>()) {
-    const TargetInfo &TI = getASTContext().getTargetInfo();
-    if (TI.hasFeature("vfp"))
-      Diag(D->getLocation(), diag::warn_arm_interrupt_vfp_clobber);
-  }
+  const TargetInfo &TI = getASTContext().getTargetInfo();
+  if (TI.hasFeature("vfp"))
+    Diag(D->getLocation(), diag::warn_arm_interrupt_vfp_clobber);
 
   D->addAttr(::new (getASTContext())
                  ARMInterruptAttr(getASTContext(), AL, Kind));

clang/lib/Sema/SemaDeclAttr.cpp

@@ -5993,20 +5993,6 @@ static void handleAbiTagAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
                  AbiTagAttr(S.Context, AL, Tags.data(), Tags.size()));
 }
 
-static void handleARMInterruptSaveFPAttr(Sema &S, Decl *D,
-                                         const ParsedAttr &AL) {
-  handleARMInterruptAttr(S, D, AL);
-
-  bool VFP = S.Context.getTargetInfo().hasFeature("vfp");
-
-  if (!VFP) {
-    S.Diag(D->getLocation(), diag::warn_arm_interrupt_save_fp_without_vfp_unit);
-    return;
-  }
-
-  D->addAttr(::new (S.Context) ARMSaveFPAttr(S.Context, AL));
-}
-
 static bool hasBTFDeclTagAttr(Decl *D, StringRef Tag) {
   for (const auto *I : D->specific_attrs<BTFDeclTagAttr>()) {
     if (I->getBTFDeclTag() == Tag)
@@ -6911,9 +6897,6 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
   case ParsedAttr::AT_Interrupt:
     handleInterruptAttr(S, D, AL);
     break;
-  case ParsedAttr::AT_ARMInterruptSaveFP:
-    handleARMInterruptSaveFPAttr(S, D, AL);
-    break;
   case ParsedAttr::AT_X86ForceAlignArgPointer:
     S.X86().handleForceAlignArgPointerAttr(D, AL);
     break;

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

@@ -3,7 +3,7 @@
 
 
 #ifdef __ARM_FP
-__attribute__((interrupt("IRQ"))) void float_irq(void); // expected-warning {{interrupt service routine with vfp enabled may clobber the interruptee's vfp state; consider using the `interrupt_save_fp` attribute to prevent this behavior}}
+__attribute__((interrupt("IRQ"))) void float_irq(void); // expected-warning {{interrupt service routine with vfp enabled may clobber the interruptee's vfp state}}
 #else // !defined(__ARM_FP)
 __attribute__((interrupt("irq"))) void foo1(void) {} // expected-warning {{'interrupt' attribute argument not supported: irq}}
 __attribute__((interrupt(IRQ))) void foo(void) {} // expected-error {{'interrupt' attribute requires a string}}

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

@@ -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], [], [IntrReadMem]>;
+                       DefaultAttrsIntrinsic<[llvm_i32_ty], [], []>;
 def int_arm_set_fpscr : ClangBuiltin<"__builtin_arm_set_fpscr">,
                        DefaultAttrsIntrinsic<[], [llvm_i32_ty], []>;
 def int_arm_vcvtr : DefaultAttrsIntrinsic<[llvm_float_ty],

clang/test/Sema/arm-interrupt-attr.c

@@ -1207,14 +1207,6 @@ 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();
@@ -1381,14 +1373,6 @@ 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.  However, we can't issue annotations
-        // for FP status registers because ".save" requires GPR registers, and
-        // ".vsave" requires DPR registers, so don't record the copy and simply
-        // emit annotations for the source registers used for the store.
-        break;
       case ARM::tLDRpci: {
         // Grab the constpool index and check, whether it corresponds to
         // original or cloned constpool entry.

clang/test/Sema/arm-interrupt-save-fp-attr.c

@@ -80,47 +80,13 @@ ARMBaseRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
                                        ? CSR_ATPCS_SplitPush_SwiftTail_SaveList
                                        : CSR_AAPCS_SwiftTail_SaveList);
   } else if (F.hasFnAttribute("interrupt")) {
-
-    // 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 PushPopSplit == ARMSubtarget::SplitR7
-          ? CSR_ATPCS_SplitPush_SaveList
-          : CSR_AAPCS_SaveList;
-      }
-    }
-
-    if (F.getFnAttribute("interrupt").getValueAsString() == "FIQ") {
+      return PushPopSplit == ARMSubtarget::SplitR7
+                 ? CSR_ATPCS_SplitPush_SaveList
+                 : CSR_AAPCS_SaveList;
+    } else 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.
       return CSR_FIQ_SaveList;