[ARM] Save floating point registers with save_fp function attribute

jakevossen5 · Benson Chu · commit bf936c53e3a2 · 2024-09-03T15:44:51.000-05: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
@@ -970,6 +970,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
@@ -2295,6 +2295,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
@@ -338,10 +338,16 @@ 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 err_arm_interrupt_called : Error<
   "interrupt service routine cannot be called directly">;
+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<Extra>;
 def warn_interrupt_attribute_invalid : Warning<
    "%select{MIPS|MSP430|RISC-V}0 'interrupt' attribute only applies to "
    "functions that have %select{no parameters|a 'void' return type}1">,
diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp
@@ -186,6 +186,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
@@ -1331,9 +1331,11 @@ 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));
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5498,6 +5498,20 @@ 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)
@@ -6284,9 +6298,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
       !Options.IncludeCXX11Attributes)
     return;
 
-  // Unknown attributes are automatically warned on. Target-specific attributes
-  // which do not apply to the current target architecture are treated as
-  // though they were unknown attributes.
+  // Unknown attributes are automatically warned on. Target-specific
+  // attributes which do not apply to the current target architecture are
+  // treated as though they were unknown attributes.
   if (AL.getKind() == ParsedAttr::UnknownAttribute ||
       !AL.existsInTarget(S.Context.getTargetInfo())) {
     S.Diag(AL.getLoc(),
@@ -6391,6 +6405,9 @@ 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;
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,59 @@
+// RUN: %clang_cc1 %s -triple arm-apple-darwin  -target-feature +vfp2 -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple thumb-apple-darwin  -target-feature +vfp3 -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple armeb-none-eabi  -target-feature +vfp4 -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple thumbeb-none-eabi  -target-feature +neon -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple thumbeb-none-eabi -target-feature +neon -target-feature +soft-float -DSOFT -verify -fsyntax-only
+
+#ifndef SOFT
+__attribute__((interrupt_save_fp(IRQ))) void foo() {} // expected-error {{'interrupt_save_fp' attribute requires a string}}
+__attribute__((interrupt_save_fp("irq"))) void foo1() {} // expected-warning {{'interrupt_save_fp' attribute argument not supported: irq}}
+
+__attribute__((interrupt_save_fp("IRQ", 1))) void foo2() {} // expected-error {{'interrupt_save_fp' attribute takes no more than 1 argument}}
+__attribute__((interrupt_save_fp("IRQ"))) void foo3() {}
+__attribute__((interrupt_save_fp("FIQ"))) void foo4() {}
+__attribute__((interrupt_save_fp("SWI"))) void foo5() {}
+__attribute__((interrupt_save_fp("ABORT"))) void foo6() {}
+__attribute__((interrupt_save_fp("UNDEF"))) void foo7() {}
+__attribute__((interrupt_save_fp)) void foo8() {}
+__attribute__((interrupt_save_fp())) void foo9() {}
+__attribute__((interrupt_save_fp(""))) void foo10() {}
+void callee1();
+__attribute__((interrupt_save_fp("IRQ"))) void callee2();
+void caller1() {
+  callee1();
+  callee2();
+}
+__attribute__((interrupt_save_fp("IRQ"))) void caller2() {
+  callee1();
+  callee2();
+}
+
+void (*callee3)();
+__attribute__((interrupt_save_fp("IRQ"))) void caller3() {
+  callee3();
+}
+#else
+__attribute__((interrupt_save_fp("IRQ"))) void foo3() {} // 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}}
+__attribute__((interrupt_save_fp("FIQ"))) void foo4() {} // 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}}
+__attribute__((interrupt_save_fp("SWI"))) void foo5() {} // 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}}
+__attribute__((interrupt_save_fp("ABORT"))) void foo6() {} // 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}}
+__attribute__((interrupt_save_fp("UNDEF"))) void foo7() {} // 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}}
+__attribute__((interrupt_save_fp)) void foo8() {} // 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}}
+__attribute__((interrupt_save_fp())) void foo9() {} // 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}}
+__attribute__((interrupt_save_fp(""))) void foo10() {} // 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}}
+void callee1();
+__attribute__((interrupt_save_fp("IRQ"))) void callee2(); // 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}}
+void caller1() {
+  callee1();
+  callee2();
+}
+__attribute__((interrupt_save_fp("IRQ"))) void caller2() { // 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}}
+  callee1();
+  callee2();
+}
+
+void (*callee3)();
+__attribute__((interrupt_save_fp("IRQ"))) void caller3() { // 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}}
+  callee3();
+}
+#endif
diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
@@ -79,18 +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();
+
     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 UseSplitPush ? 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 UseSplitPush ? CSR_AAPCS_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)>;
@@ -357,6 +373,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.
 
@@ -365,4 +388,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
