From dc83a3c44940ca4d13839613dc55049e5eddc42e Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham@arm.com>
Date: Mon, 3 Jun 2019 11:02:53 +0000
Subject: [PATCH] [ARM] Fix recent breakage of -mfpu=none.

The recent change 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) `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.

Reviewers: SjoerdMeijer, dmgreen

Reviewed By: dmgreen

Subscribers: lebedev.ri, javed.absar, kristof.beyls, hiraditya, cfe-commits, llvm-commits

Tags: #clang, #llvm

Differential Revision: https://reviews.llvm.org/D62729

llvm-svn: 362380
---
 clang/lib/Driver/ToolChains/Arch/ARM.cpp | 9 +++++++--
 clang/test/CodeGen/arm-mfpu-none.c       | 8 ++++++++
 clang/test/Driver/arm-mfpu.c             | 2 ++
 llvm/lib/Support/ARMTargetParser.cpp     | 1 +
 4 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/CodeGen/arm-mfpu-none.c

diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 77fad7ed68f91..e38ce4d583fa7 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -430,8 +430,8 @@ void arm::getARMTargetFeatures(const ToolChain &TC,
     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.
@@ -439,6 +439,11 @@ void arm::getARMTargetFeatures(const ToolChain &TC,
                                 "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.
diff --git a/clang/test/CodeGen/arm-mfpu-none.c b/clang/test/CodeGen/arm-mfpu-none.c
new file mode 100644
index 0000000000000..ae4b07d863813
--- /dev/null
+++ b/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);
+}
diff --git a/clang/test/Driver/arm-mfpu.c b/clang/test/Driver/arm-mfpu.c
index 33cad80bf1492..921afa227ce28 100644
--- a/clang/test/Driver/arm-mfpu.c
+++ b/clang/test/Driver/arm-mfpu.c
@@ -318,6 +318,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FP %s
 // CHECK-NO-FP-NOT: "-target-feature" "+soft-float"
 // CHECK-NO-FP: "-target-feature" "+soft-float-abi"
+// CHECK-NO-FP: "-target-feature" "-fpregs"
 // CHECK-NO-FP: "-target-feature" "-vfp2"
 // CHECK-NO-FP: "-target-feature" "-vfp3"
 // CHECK-NO-FP: "-target-feature" "-vfp4"
@@ -363,6 +364,7 @@
 // CHECK-SOFT-ABI-FP: "-target-feature" "-fp-armv8"
 // CHECK-SOFT-ABI-FP: "-target-feature" "-neon"
 // CHECK-SOFT-ABI-FP: "-target-feature" "-crypto"
+// CHECK-SOFT-ABI-FP: "-target-feature" "-fpregs"
 
 // RUN: %clang -target arm-linux-androideabi21 %s -### -c 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-ARM5-ANDROID-FP-DEFAULT %s
diff --git a/llvm/lib/Support/ARMTargetParser.cpp b/llvm/lib/Support/ARMTargetParser.cpp
index a33f602e532bf..8806ea52fdf42 100644
--- a/llvm/lib/Support/ARMTargetParser.cpp
+++ b/llvm/lib/Support/ARMTargetParser.cpp
@@ -198,6 +198,7 @@ bool ARM::getFPUFeatures(unsigned FPUKind, std::vector<StringRef> &Features) {
     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");