[ARM] Save floating point registers with save_fp function attribute

jakevossen5 · Benson Chu · commit a935c458c71f · 2024-12-27T16:59:53.000-06:00
[ARM] and interrupt_save_fp attribute

interupt_save_fp update name; fix bugs

[ARM] fix typos and register class name

used better push / pop instructions

change epilog emitting order

WIP with FPSCR

save just d regs

cleaned up docs and ARMRegisterInfo td

change m3 to m4

fix reg tests

Minor format changes on top of Jake Vossen's support for
interrupt_save_fp function attribute which preserves VFP D
registers at the moment. FPSCR and FPEXC registers to follow.
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
@@ -984,6 +984,22 @@ 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]>;
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
@@ -2484,6 +2484,20 @@ 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. 
+If the FPEXC or FPSCR are needed, that state must be saved manually. 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 = [{
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -346,8 +346,14 @@ 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">,
+  "interruptee's vfp state; "
+  "consider using the `interrupt_save_fp` attribute to prevent this behavior">,
   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-save-fp-no-vfp-unit">>;
 def err_arm_interrupt_called : Error<
   "interrupt service routine cannot be called directly">;
 def warn_interrupt_attribute_invalid : Warning<
diff --git a/clang/include/clang/Sema/SemaARM.h b/clang/include/clang/Sema/SemaARM.h
@@ -79,6 +79,7 @@ class SemaARM : public SemaBase {
   void handleNewAttr(Decl *D, const ParsedAttr &AL);
   void handleCmseNSEntryAttr(Decl *D, const ParsedAttr &AL);
   void handleInterruptAttr(Decl *D, const ParsedAttr &AL);
+  void handleInterruptSaveFPAttr(Decl *D, const ParsedAttr &AL);
 };
 
 SemaARM::ArmStreamingType getArmStreamingFnType(const FunctionDecl *FD);
diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp
@@ -189,6 +189,12 @@ 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;
diff --git a/clang/lib/Sema/SemaARM.cpp b/clang/lib/Sema/SemaARM.cpp
@@ -1320,12 +1320,37 @@ void SemaARM::handleInterruptAttr(Decl *D, const ParsedAttr &AL) {
     return;
   }
 
-  const TargetInfo &TI = getASTContext().getTargetInfo();
-  if (TI.hasFeature("vfp"))
-    Diag(D->getLocation(), diag::warn_arm_interrupt_vfp_clobber);
+  if (!D->hasAttr<ARMSaveFPAttr>()) {
+    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));
 }
 
+void SemaARM::handleInterruptSaveFPAttr(Decl *D, const ParsedAttr &AL) {
+  // Go ahead and add ARMSaveFPAttr because handleInterruptAttr() checks for
+  // it when deciding to issue a diagnostic about clobbering floating point
+  // registers, which ARMSaveFPAttr prevents.
+  D->addAttr(::new (SemaRef.Context) ARMSaveFPAttr(SemaRef.Context, AL));
+  SemaRef.ARM().handleInterruptAttr(D, AL);
+
+  // If ARM().handleInterruptAttr() failed, remove ARMSaveFPAttr.
+  if (!D->hasAttr<ARMInterruptAttr>()) {
+    D->dropAttr<ARMSaveFPAttr>();
+    return;
+  }
+
+  // If VFP not enabled, remove ARMSaveFPAttr but leave ARMInterruptAttr.
+  bool VFP = SemaRef.Context.getTargetInfo().hasFeature("vfp");
+
+  if (!VFP) {
+    SemaRef.Diag(D->getLocation(), diag::warn_arm_interrupt_save_fp_without_vfp_unit);
+    D->dropAttr<ARMSaveFPAttr>();
+    return;
+  }
+}
+
 } // namespace clang
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6645,6 +6645,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
   case ParsedAttr::AT_Interrupt:
     handleInterruptAttr(S, D, AL);
     break;
+  case ParsedAttr::AT_ARMInterruptSaveFP:
+    S.ARM().handleInterruptSaveFPAttr(D, AL);
+    break;
   case ParsedAttr::AT_X86ForceAlignArgPointer:
     S.X86().handleForceAlignArgPointerAttr(D, AL);
     break;
diff --git a/clang/test/CodeGen/arm-interrupt-save-fp-attr.c b/clang/test/CodeGen/arm-interrupt-save-fp-attr.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -triple thumb-apple-darwin -target-abi aapcs -target-feature +vfp4 -target-cpu cortex-m3 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm-apple-darwin -target-abi apcs-gnu -target-feature +vfp4 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-APCS
+
+__attribute__((interrupt_save_fp)) void test_generic_interrupt() {
+  // CHECK: define{{.*}} arm_aapcscc void @test_generic_interrupt() [[GENERIC_ATTR:#[0-9]+]]
+
+  // CHECK-APCS: define{{.*}} void @test_generic_interrupt() [[GENERIC_ATTR:#[0-9]+]]
+}
+
+__attribute__((interrupt_save_fp("IRQ"))) void test_irq_interrupt() {
+  // CHECK: define{{.*}} arm_aapcscc void @test_irq_interrupt() [[IRQ_ATTR:#[0-9]+]]
+}
+
+__attribute__((interrupt_save_fp("FIQ"))) void test_fiq_interrupt() {
+  // CHECK: define{{.*}} arm_aapcscc void @test_fiq_interrupt() [[FIQ_ATTR:#[0-9]+]]
+}
+
+__attribute__((interrupt_save_fp("SWI"))) void test_swi_interrupt() {
+  // CHECK: define{{.*}} arm_aapcscc void @test_swi_interrupt() [[SWI_ATTR:#[0-9]+]]
+}
+
+__attribute__((interrupt_save_fp("ABORT"))) void test_abort_interrupt() {
+  // CHECK: define{{.*}} arm_aapcscc void @test_abort_interrupt() [[ABORT_ATTR:#[0-9]+]]
+}
+
+
+__attribute__((interrupt_save_fp("UNDEF"))) void test_undef_interrupt() {
+  // CHECK: define{{.*}} arm_aapcscc void @test_undef_interrupt() [[UNDEF_ATTR:#[0-9]+]]
+}
+
+
+// CHECK: attributes [[GENERIC_ATTR]] = { {{.*}} {{"interrupt"[^=]}}{{.*}} "save-fp"
+// CHECK: attributes [[IRQ_ATTR]] = { {{.*}} "interrupt"="IRQ" {{.*}} "save-fp"
+// CHECK: attributes [[FIQ_ATTR]] = { {{.*}} "interrupt"="FIQ" {{.*}} "save-fp"
+// CHECK: attributes [[SWI_ATTR]] = { {{.*}} "interrupt"="SWI" {{.*}} "save-fp"
+// CHECK: attributes [[ABORT_ATTR]] = { {{.*}} "interrupt"="ABORT" {{.*}} "save-fp"
+// CHECK: attributes [[UNDEF_ATTR]] = { {{.*}} "interrupt"="UNDEF" {{.*}} "save-fp"
+
+// CHECK-APCS: attributes [[GENERIC_ATTR]] = { {{.*}} "interrupt" {{.*}} "save-fp"
diff --git a/clang/test/Sema/arm-interrupt-attr.c b/clang/test/Sema/arm-interrupt-attr.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}}
+__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}}
 #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}}
diff --git a/clang/test/Sema/arm-interrupt-save-fp-attr.c b/clang/test/Sema/arm-interrupt-save-fp-attr.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -triple arm-none-eabi -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple arm-none-eabi -target-feature +vfp2 -verify -fsyntax-only
+
+
+#if !defined(__ARM_FP)
+__attribute__((interrupt_save_fp("IRQ"))) void float_irq(void); // expected-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}}
+#else // defined(__ARM_FP)
+__attribute__((interrupt_save_fp("irq"))) void foo1(void) {} // expected-warning {{'interrupt_save_fp' attribute argument not supported: irq}}
+__attribute__((interrupt_save_fp(IRQ))) void foo(void) {} // expected-error {{'interrupt_save_fp' attribute requires a string}}
+__attribute__((interrupt_save_fp("IRQ", 1))) void foo2(void) {} // expected-error {{'interrupt_save_fp' attribute takes no more than 1 argument}}
+__attribute__((interrupt_save_fp("IRQ"))) void foo3(void) {}
+__attribute__((interrupt_save_fp("FIQ"))) void foo4(void) {}
+__attribute__((interrupt_save_fp("SWI"))) void foo5(void) {}
+__attribute__((interrupt_save_fp("ABORT"))) void foo6(void) {}
+__attribute__((interrupt_save_fp("UNDEF"))) void foo7(void) {}
+__attribute__((interrupt_save_fp)) void foo8(void) {}
+__attribute__((interrupt_save_fp())) void foo9(void) {}
+__attribute__((interrupt_save_fp(""))) void foo10(void) {}
+
+__attribute__((interrupt_save_fp("IRQ"))) void callee(void) {}
+
+void caller(void)
+{
+    callee(); // expected-error {{interrupt service routine cannot be called directly}}
+}
+#endif // __ARM_FP
diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
@@ -80,20 +80,44 @@ ARMBaseRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
                                        ? 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();
+
     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.
-      return PushPopSplit == ARMSubtarget::SplitR7
-                 ? CSR_ATPCS_SplitPush_SaveList
-                 : CSR_AAPCS_SaveList;
+      // 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;
+      }
     } 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;
+      if (SaveFP) {
+        return HasNEON ? CSR_FIQ_FP_NEON_SaveList : CSR_FIQ_FP_SaveList;
+      } else {
+        return CSR_FIQ_SaveList;
+      }
     } else {
       // Generally only R13-R14 (i.e. SP, LR) are automatically preserved by
       // exception handling.
-      return CSR_GenericInt_SaveList;
+      if (SaveFP) {
+        return HasNEON ? CSR_GenericInt_FP_NEON_SaveList
+                       : CSR_GenericInt_FP_SaveList;
+      } else {
+        return CSR_GenericInt_SaveList;
+      }
     }
   }
 
diff --git a/llvm/lib/Target/ARM/ARMCallingConv.td b/llvm/lib/Target/ARM/ARMCallingConv.td
@@ -267,9 +267,18 @@ 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_NEON_Interrupt_Regs : CalleeSavedRegs<(add CSR_FP_Interrupt_Regs, 
+                                                  (sequence "D%u", 31, 16))>;
+
 def CSR_AAPCS : CalleeSavedRegs<(add LR, R11, R10, R9, R8, R7, R6, R5, R4,
                                      (sequence "D%u", 15, 8))>;
 
+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,
@@ -293,6 +302,13 @@ 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, 
+                                              CSR_FP_Interrupt_Regs)>;
+
+def CSR_AAPCS_SplitPush_FP_NEON : CalleeSavedRegs<(add CSR_AAPCS_SplitPush_FP, 
+                                                   CSR_FP_NEON_Interrupt_Regs)>;
+
+
 // R8 is used to pass swifterror, remove it from CSR.
 def CSR_ATPCS_SplitPush_SwiftError : CalleeSavedRegs<(sub CSR_ATPCS_SplitPush,
                                                       R8)>;
@@ -360,6 +376,13 @@ def CSR_iOS_CXX_TLS_ViaCopy : CalleeSavedRegs<(sub CSR_iOS_CXX_TLS,
 // generally does rather than tracking its liveness as a normal register.
 def CSR_GenericInt : CalleeSavedRegs<(add LR, (sequence "R%u", 12, 0))>;
 
+def CSR_GenericInt_FP : CalleeSavedRegs<(add CSR_GenericInt, 
+                                         CSR_FP_Interrupt_Regs)>;
+
+def CSR_GenericInt_FP_NEON : CalleeSavedRegs<(add CSR_GenericInt_FP, 
+                                              CSR_FP_NEON_Interrupt_Regs)>;
+
+
 // The fast interrupt handlers have more private state and get their own copies
 // of R8-R12, in addition to SP and LR. As before, mark LR for saving too.
 
@@ -368,4 +391,9 @@ def CSR_GenericInt : CalleeSavedRegs<(add LR, (sequence "R%u", 12, 0))>;
 // registers.
 def CSR_FIQ : CalleeSavedRegs<(add LR, R11, (sequence "R%u", 7, 0))>;
 
+def CSR_FIQ_FP : CalleeSavedRegs<(add CSR_FIQ, CSR_FP_Interrupt_Regs)>;
+
+def CSR_FIQ_FP_NEON : CalleeSavedRegs<(add CSR_FIQ_FP, 
+                                       CSR_FP_NEON_Interrupt_Regs)>;
+
 
diff --git a/llvm/test/CodeGen/ARM/interrupt-save-fp-attr.ll b/llvm/test/CodeGen/ARM/interrupt-save-fp-attr.ll
