simon_tatham created this revision.
simon_tatham added reviewers: SjoerdMeijer, dmgreen.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, kristof.beyls,
javed.absar.
Herald added projects: clang, LLVM.
The recent change D60691 <https://reviews.llvm.org/D60691> introduced a bug in
clang when handling
option combinations such as `-mcpu=cortex-m4 -mfpu=none`. Those
options together should select Cortex-M4 but disable all use of
hardware FP, but in fact, now hardware FP instructions can still be
generated in that mode.
The reason is because the handling of FPUVersion::NONE disables all
the same feature names it used to, of which the base one is `vfp2`.
But now there are further features below that, like `vfp2d16fp` and
(following D60694 <https://reviews.llvm.org/D60694>) `fpregs`, which also need
to be turned off to
disable hardware FP completely.
Added a tiny test which double-checks that compiling a simple FP
function doesn't access the FP registers.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D62729
Files:
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/test/CodeGen/arm-mfpu-none.c
llvm/lib/Support/ARMTargetParser.cpp
Index: llvm/lib/Support/ARMTargetParser.cpp
===================================================================
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -198,6 +198,7 @@
Features.push_back("-fp-armv8");
break;
case FPUVersion::NONE:
+ Features.push_back("-fpregs");
Features.push_back("-vfp2");
Features.push_back("-vfp3");
Features.push_back("-fp16");
Index: clang/test/CodeGen/arm-mfpu-none.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/arm-mfpu-none.c
@@ -0,0 +1,8 @@
+// REQUIRES: arm-registered-target
+// RUN: %clang -target arm-none-eabi -mcpu=cortex-m4 -mfpu=none -S -o - %s |
FileCheck %s
+
+// CHECK-LABEL: compute
+// CHECK-NOT: {{s[0-9]}}
+float compute(float a, float b) {
+ return (a+b) * (a-b);
+}
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -433,8 +433,8 @@
llvm::ARM::getFPUFeatures(llvm::ARM::FK_NONE, Features);
// Disable hardware FP features which have been enabled.
- // FIXME: Disabling vfp2 and neon should be enough as all the other
- // features are dependent on these 2 features in LLVM. However
+ // FIXME: Disabling fpregs should be enough all by itself, since all
+ // the other FP features are dependent on it. However
// there is currently no easy way to test this in clang, so for
// now just be explicit and disable all known dependent features
// as well.
@@ -442,6 +442,11 @@
"neon", "crypto", "dotprod", "fp16fml"})
if (std::find(std::begin(Features), std::end(Features), "+" + Feature)
!= std::end(Features))
Features.push_back(Args.MakeArgString("-" + Feature));
+
+ // Disable the base feature unconditionally, even if it was not
+ // explicitly in the features list (e.g. if we had +vfp3, which
+ // implies it).
+ Features.push_back("-fpregs");
}
// En/disable crc code generation.
Index: llvm/lib/Support/ARMTargetParser.cpp
===================================================================
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -198,6 +198,7 @@
Features.push_back("-fp-armv8");
break;
case FPUVersion::NONE:
+ Features.push_back("-fpregs");
Features.push_back("-vfp2");
Features.push_back("-vfp3");
Features.push_back("-fp16");
Index: clang/test/CodeGen/arm-mfpu-none.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/arm-mfpu-none.c
@@ -0,0 +1,8 @@
+// REQUIRES: arm-registered-target
+// RUN: %clang -target arm-none-eabi -mcpu=cortex-m4 -mfpu=none -S -o - %s | FileCheck %s
+
+// CHECK-LABEL: compute
+// CHECK-NOT: {{s[0-9]}}
+float compute(float a, float b) {
+ return (a+b) * (a-b);
+}
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -433,8 +433,8 @@
llvm::ARM::getFPUFeatures(llvm::ARM::FK_NONE, Features);
// Disable hardware FP features which have been enabled.
- // FIXME: Disabling vfp2 and neon should be enough as all the other
- // features are dependent on these 2 features in LLVM. However
+ // FIXME: Disabling fpregs should be enough all by itself, since all
+ // the other FP features are dependent on it. However
// there is currently no easy way to test this in clang, so for
// now just be explicit and disable all known dependent features
// as well.
@@ -442,6 +442,11 @@
"neon", "crypto", "dotprod", "fp16fml"})
if (std::find(std::begin(Features), std::end(Features), "+" + Feature) != std::end(Features))
Features.push_back(Args.MakeArgString("-" + Feature));
+
+ // Disable the base feature unconditionally, even if it was not
+ // explicitly in the features list (e.g. if we had +vfp3, which
+ // implies it).
+ Features.push_back("-fpregs");
}
// En/disable crc code generation.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
