[ARM] Save floating point registers and status registers with save_fp function attribute #89654
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-backend-aarch64 @llvm/pr-subscribers-clang Author: Benson Chu (pestctrl) ChangesThe interrupt attribute currently doesn't save floating-point related registers in the frame setup and destroy. This patch adds a new interrupt_save_fp attribute that will save floating-point registers as well. Patch is 76.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/89654.diff 22 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index dc87a8c6f022dc..71775ce1e12461 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -914,6 +914,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",
+ ["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 = [ARMInterruptSaveFPDocs];
+}
+
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
index a0bbe5861c5722..1cd642ba90aa9e 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -2239,6 +2239,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
index 63e951daec7477..1f519d0fc1cc35 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -335,7 +335,14 @@ def warn_anyx86_excessive_regsave : Warning<
" or be compiled with '-mgeneral-regs-only'">,
InGroup<DiagGroup<"excessive-regsave">>;
def warn_arm_interrupt_calling_convention : Warning<
- "call to function without interrupt attribute could clobber interruptee's VFP registers">,
+ "call to function without interrupt attribute could clobber interruptee's "
+ "VFP registers; consider using the `interrupt_save_fp` attribute to prevent "
+ "this behavior">,
+ InGroup<Extra>;
+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 "
diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp
index 885d9c77d0e76f..a45858e2e2e586 100644
--- a/clang/lib/CodeGen/Targets/ARM.cpp
+++ b/clang/lib/CodeGen/Targets/ARM.cpp
@@ -185,6 +185,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/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 363ae93cb62df1..086f0c8daa36c3 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7524,6 +7524,19 @@ static void handleARMInterruptAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
D->addAttr(::new (S.Context) ARMInterruptAttr(S.Context, AL, Kind));
}
+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 void handleMSP430InterruptAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
// MSP430 'interrupt' attribute is applied to
// a function with no parameters and void return type.
@@ -9134,9 +9147,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
if (AL.isCXX11Attribute() && !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(),
@@ -9145,7 +9158,7 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
: AL.isDeclspecAttribute()
? (unsigned)diag::warn_unhandled_ms_attribute_ignored
: (unsigned)diag::warn_unknown_attribute_ignored)
- << AL << AL.getRange();
+ << AL << AL.getRange();
return;
}
@@ -9241,6 +9254,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:
handleX86ForceAlignArgPointerAttr(S, D, AL);
break;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 5c861467bc1023..72be89dbc53cf3 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6931,7 +6931,8 @@ ExprResult Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl,
// no_caller_saved_registers since there is no efficient way to
// save and restore the non-GPR state.
if (auto *Caller = getCurFunctionDecl()) {
- if (Caller->hasAttr<ARMInterruptAttr>()) {
+ if (Caller->hasAttr<ARMInterruptAttr>() &&
+ !Caller->hasAttr<ARMSaveFPAttr>()) {
bool VFP = Context.getTargetInfo().hasFeature("vfp");
if (VFP && (!FDecl || !FDecl->hasAttr<ARMInterruptAttr>())) {
Diag(Fn->getExprLoc(), diag::warn_arm_interrupt_calling_convention);
diff --git a/clang/test/CodeGen/arm-interrupt-save-fp-attr-status-regs.c b/clang/test/CodeGen/arm-interrupt-save-fp-attr-status-regs.c
new file mode 100644
index 00000000000000..457f725f8d3af6
--- /dev/null
+++ b/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();
+}
diff --git a/clang/test/CodeGen/arm-interrupt-save-fp-attr.c b/clang/test/CodeGen/arm-interrupt-save-fp-attr.c
new file mode 100644
index 00000000000000..5db8b3daa72126
--- /dev/null
+++ 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"
\ No newline at end of file
diff --git a/clang/test/Sema/arm-interrupt-attr.c b/clang/test/Sema/arm-interrupt-attr.c
index 3537fba8521ad9..937fd929483da0 100644
--- a/clang/test/Sema/arm-interrupt-attr.c
+++ b/clang/test/Sema/arm-interrupt-attr.c
@@ -31,13 +31,13 @@ void caller1(void) {
#ifndef SOFT
__attribute__((interrupt("IRQ"))) void caller2(void) {
- callee1(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
+ callee1(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers; consider using the `interrupt_save_fp` attribute to prevent this behavior}}
callee2();
}
void (*callee3)(void);
__attribute__((interrupt("IRQ"))) void caller3(void) {
- callee3(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
+ callee3(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers; consider using the `interrupt_save_fp` attribute to prevent this behavior}}
}
#else
__attribute__((interrupt("IRQ"))) void caller2(void) {
diff --git a/clang/test/Sema/arm-interrupt-save-fp-attr.c b/clang/test/Sema/arm-interrupt-save-fp-attr.c
new file mode 100644
index 00000000000000..e0fd4e2c4d1288
--- /dev/null
+++ 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
\ No newline at end of file
diff --git a/llvm/include/llvm/IR/IntrinsicsARM.td b/llvm/include/llvm/IR/IntrinsicsARM.td
index 11b9877091a8ed..91f9d40f2e05f0 100644
--- a/llvm/include/llvm/IR/IntrinsicsARM.td
+++ b/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],
diff --git a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
index 642739a29d6b06..353f5d1a57540d 100644
--- a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
+++ b/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.
diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
index 9adf758b46c481..ed774ea185f855 100644
--- a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
@@ -79,11 +79,35 @@ ARMBaseRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
: (UseSplitPush ? 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()) {
...
[truncated]
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions c,cpp,h -- clang/test/CodeGen/arm-interrupt-save-fp-attr-status-regs.c clang/test/CodeGen/arm-interrupt-save-fp-attr.c clang/test/Sema/arm-interrupt-save-fp-attr.c clang/include/clang/Sema/SemaARM.h clang/lib/CodeGen/Targets/ARM.cpp clang/lib/Sema/SemaARM.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/test/Sema/arm-interrupt-attr.c llvm/lib/Target/ARM/ARMAsmPrinter.cpp llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp llvm/lib/Target/ARM/ARMFrameLowering.cpp llvm/lib/Target/ARM/ARMFrameLowering.h llvm/lib/Target/ARM/ARMMachineFunctionInfo.hView the diff from clang-format here.diff --git a/clang/lib/Sema/SemaARM.cpp b/clang/lib/Sema/SemaARM.cpp
index c1cfea042..dd84ff2f4 100644
--- a/clang/lib/Sema/SemaARM.cpp
+++ b/clang/lib/Sema/SemaARM.cpp
@@ -1343,7 +1343,8 @@ void SemaARM::handleInterruptSaveFPAttr(Decl *D, const ParsedAttr &AL) {
bool VFP = SemaRef.Context.getTargetInfo().hasFeature("vfp");
if (!VFP) {
- SemaRef.Diag(D->getLocation(), diag::warn_arm_interrupt_save_fp_without_vfp_unit);
+ SemaRef.Diag(D->getLocation(),
+ diag::warn_arm_interrupt_save_fp_without_vfp_unit);
D->dropAttr<ARMSaveFPAttr>();
}
}
diff --git a/llvm/lib/Target/ARM/ARMFrameLowering.cpp b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
index 64c067f2b..be29ece61 100644
--- a/llvm/lib/Target/ARM/ARMFrameLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
@@ -913,8 +913,8 @@ void ARMFrameLowering::emitPrologue(MachineFunction &MF,
// Determine the sizes of each callee-save spill areas and record which frame
// belongs to which callee-save spill areas.
- unsigned GPRCS1Size = 0, GPRCS2Size = 0, FPStatusSize = 0,
- DPRCS1Size = 0, GPRCS3Size = 0, DPRCS2Size = 0;
+ unsigned GPRCS1Size = 0, GPRCS2Size = 0, FPStatusSize = 0, DPRCS1Size = 0,
+ GPRCS3Size = 0, DPRCS2Size = 0;
int FramePtrSpillFI = 0;
int D8SpillFI = 0;
@@ -1844,9 +1844,8 @@ void ARMFrameLowering::emitFPStatusSaves(MachineBasicBlock &MBB,
SmallVector<MCRegister> Regs;
auto RegPresent = [&CSI](MCRegister Reg) {
- return llvm::any_of(CSI, [Reg](const CalleeSavedInfo &C) {
- return C.getReg() == Reg;
- });
+ return llvm::any_of(
+ CSI, [Reg](const CalleeSavedInfo &C) { return C.getReg() == Reg; });
};
// If we need to save FPSCR, then we must move FPSCR into R4 with the VMRS
@@ -1893,9 +1892,8 @@ void ARMFrameLowering::emitFPStatusRestores(
SmallVector<MCRegister> Regs;
auto RegPresent = [&CSI](MCRegister Reg) {
- return llvm::any_of(CSI, [Reg](const CalleeSavedInfo &C) {
- return C.getReg() == Reg;
- });
+ return llvm::any_of(
+ CSI, [Reg](const CalleeSavedInfo &C) { return C.getReg() == Reg; });
};
// Do nothing if we don't need to restore any FP status registers.
diff --git a/llvm/lib/Target/ARM/ARMMachineFunctionInfo.h b/llvm/lib/Target/ARM/ARMMachineFunctionInfo.h
index a9c4b53f9..7692b7cdf 100644
--- a/llvm/lib/Target/ARM/ARMMachineFunctionInfo.h
+++ b/llvm/lib/Target/ARM/ARMMachineFunctionInfo.h
@@ -206,7 +206,7 @@ public:
unsigned getFrameRecordSavedAreaSize() const { return FRSaveSize; }
unsigned getGPRCalleeSavedArea1Size() const { return GPRCS1Size; }
unsigned getGPRCalleeSavedArea2Size() const { return GPRCS2Size; }
- unsigned getFPStatusSavesSize() const { return FPStatusSize; }
+ unsigned getFPStatusSavesSize() const { return FPStatusSize; }
unsigned getDPRCalleeSavedGapSize() const { return DPRCSAlignGapSize; }
unsigned getDPRCalleeSavedArea1Size() const { return DPRCS1Size; }
unsigned getGPRCalleeSavedArea3Size() const { return GPRCS3Size; }
@@ -215,7 +215,7 @@ public:
void setFrameRecordSavedAreaSize(unsigned s) { FRSaveSize = s; }
void setGPRCalleeSavedArea1Size(unsigned s) { GPRCS1Size = s; }
void setGPRCalleeSavedArea2Size(unsigned s) { GPRCS2Size = s; }
- void setFPStatusSavesSize(unsigned s) { FPStatusSize = s; }
+ void setFPStatusSavesSize(unsigned s) { FPStatusSize = s; }
void setDPRCalleeSavedGapSize(unsigned s) { DPRCSAlignGapSize = s; }
void setDPRCalleeSavedArea1Size(unsigned s) { DPRCS1Size = s; }
void setGPRCalleeSavedArea3Size(unsigned s) { GPRCS3Size = s; }
|
pestctrl
force-pushed
the
save-fp-reg
branch
5 times, most recently
from
7f036d3 to
c3aad46
Compare
efriedma-quic
requested review from
ostannard and
davemgreen
and removed request for
efriedma-quic
pestctrl
force-pushed
the
save-fp-reg
branch
2 times, most recently
from
4b99dc8 to
207438b
Compare
|
ping! |
| 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 |
There was a problem hiding this comment.
This says FPSCR isn't saved... but some of the tests show FPSCR getting saved. Which is right?
There was a problem hiding this comment.
The FPSCR is in fact saved, along with the FPEXC.
|
ping! |
pestctrl
force-pushed
the
save-fp-reg
branch
from
430c81b to
9fa28b7
Compare
|
ping |
| "`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>; |
There was a problem hiding this comment.
Please don't add diagnostics to Wextra; it's a specific set of diagnostics based on the equivalent gcc flag, not a catchall.
| __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} |
There was a problem hiding this comment.
It looks like this is the only test that checks the new .save syntax? There should be tests in llvm/test/CodeGen/ARM and llvm/test/MC/ARM
There was a problem hiding this comment.
I have reverted that syntax change. We had initially rolled it out internally, and later found that our assembler didn't like that syntax change, so we rolled it back. I forgot to roll it back on the upstream branch.
To our understanding, the .save directive is for unwinding, and only accepts GPR's. .vsave isn't the correct directive to use either, since those only accept DPR's. In the end, we decided to just have those .save's refer to the actual GPR's.
There was a problem hiding this comment.
At that point, you might as well not emit the .save at all? At best, the debugger will just see the values get overwritten, and ignore the second .save.
pestctrl
force-pushed
the
save-fp-reg
branch
from
9fa28b7 to
95a48ba
Compare
clang/lib/Sema/SemaDeclAttr.cpp
Outdated
| const ParsedAttr &AL) { | ||
| handleARMInterruptAttr(S, D, AL); | ||
|
|
||
| bool VFP = S.Context.getTargetInfo().hasFeature("vfp"); |
There was a problem hiding this comment.
Do we care if someone uses a "target" attribute? Maybe not relevant for embedded targets where you'd use this.
| // 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")) { |
There was a problem hiding this comment.
It's a little weird to check isTargetHardFloat here... that just refers to whether the calling convention passes arguments in integer registers or FP registers. It doesn't have anything to do with whether floating-point registers are used.
There was a problem hiding this comment.
How about we check for if if the subtarget has FP registers?
if (STI.hasFPRegs() && F.hasFnAttribute("save-fp"))
9467409 to
51fff11
Compare
|
Is this still waiting for something from me? |
|
@efriedma-quic No, I will push this on Monday. Thank you! |
FPSCR and FPEXC will be stored in FPStatusRegs, after GPRCS2 has been saved. - GPRCS1 - GPRCS2 - FPStatusRegs (new) - DPRCS - GPRCS3 - DPRCS2 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. 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. Avoid annotating the saving of FPSCR and FPEXC for functions marked with the interrupt_save_fp attribute, even though this is done as part of frame setup. Since these are status registers, there really is no viable way of annotating this. Since these aren't GPRs or DPRs, they can't be used with .save or .vsave directives. Instead, just record that the intermediate registers r4 and r5 are saved to the stack again. Co-authored-by: Jake Vossen <[email protected]> Co-authored-by: Alan Phipps <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The interrupt attribute currently doesn't save floating-point related registers in the frame setup and destroy.
This patch adds a new interrupt_save_fp attribute that will save floating-point registers as well.