[Driver][ARM] Disable unsupported features when nofp arch extension is used

A list of target features is disabled when there is no hardware
floating-point support. This is the case when one of the following
options is passed to clang:

 - -mfloat-abi=soft
 - -mfpu=none

This option list is missing, however, the extension "+nofp" that can be
specified in -march flags, such as "-march=armv8-a+nofp".

This patch also disables unsupported target features when nofp is passed
to -march.

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

Author: vhscampos

clang/lib/Driver/ToolChains/Arch/ARM.cpp

@@ -73,14 +73,15 @@ static unsigned getARMFPUFeatures(const Driver &D, const Arg *A,
 }
 
 // Decode ARM features from string like +[no]featureA+[no]featureB+...
-static bool DecodeARMFeatures(const Driver &D, StringRef text,
-                              StringRef CPU, llvm::ARM::ArchKind ArchKind,
-                              std::vector<StringRef> &Features) {
+static bool DecodeARMFeatures(const Driver &D, StringRef text, StringRef CPU,
+                              llvm::ARM::ArchKind ArchKind,
+                              std::vector<StringRef> &Features,
+                              unsigned &ArgFPUID) {
   SmallVector<StringRef, 8> Split;
   text.split(Split, StringRef("+"), -1, false);
 
   for (StringRef Feature : Split) {
-    if (!appendArchExtFeatures(CPU, ArchKind, Feature, Features))
+    if (!appendArchExtFeatures(CPU, ArchKind, Feature, Features, ArgFPUID))
       return false;
   }
   return true;
@@ -102,30 +103,30 @@ static void DecodeARMFeaturesFromCPU(const Driver &D, StringRef CPU,
 static void checkARMArchName(const Driver &D, const Arg *A, const ArgList &Args,
                              llvm::StringRef ArchName, llvm::StringRef CPUName,
                              std::vector<StringRef> &Features,
-                             const llvm::Triple &Triple) {
+                             const llvm::Triple &Triple, unsigned &ArgFPUID) {
   std::pair<StringRef, StringRef> Split = ArchName.split("+");
 
   std::string MArch = arm::getARMArch(ArchName, Triple);
   llvm::ARM::ArchKind ArchKind = llvm::ARM::parseArch(MArch);
   if (ArchKind == llvm::ARM::ArchKind::INVALID ||
-      (Split.second.size() && !DecodeARMFeatures(
-        D, Split.second, CPUName, ArchKind, Features)))
+      (Split.second.size() && !DecodeARMFeatures(D, Split.second, CPUName,
+                                                 ArchKind, Features, ArgFPUID)))
     D.Diag(clang::diag::err_drv_clang_unsupported) << A->getAsString(Args);
 }
 
 // Check -mcpu=. Needs ArchName to handle -mcpu=generic.
 static void checkARMCPUName(const Driver &D, const Arg *A, const ArgList &Args,
                             llvm::StringRef CPUName, llvm::StringRef ArchName,
                             std::vector<StringRef> &Features,
-                            const llvm::Triple &Triple) {
+                            const llvm::Triple &Triple, unsigned &ArgFPUID) {
   std::pair<StringRef, StringRef> Split = CPUName.split("+");
 
   std::string CPU = arm::getARMTargetCPU(CPUName, ArchName, Triple);
   llvm::ARM::ArchKind ArchKind =
     arm::getLLVMArchKindForARM(CPU, ArchName, Triple);
   if (ArchKind == llvm::ARM::ArchKind::INVALID ||
-      (Split.second.size() && !DecodeARMFeatures(
-        D, Split.second, CPU, ArchKind, Features)))
+      (Split.second.size() &&
+       !DecodeARMFeatures(D, Split.second, CPU, ArchKind, Features, ArgFPUID)))
     D.Diag(clang::diag::err_drv_clang_unsupported) << A->getAsString(Args);
 }
 
@@ -347,6 +348,8 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
   const Arg *CPUArg = Args.getLastArg(options::OPT_mcpu_EQ);
   StringRef ArchName;
   StringRef CPUName;
+  unsigned ArchArgFPUID = llvm::ARM::FK_INVALID;
+  unsigned CPUArgFPUID = llvm::ARM::FK_INVALID;
 
   // Check -mcpu. ClangAs gives preference to -Wa,-mcpu=.
   if (WaCPU) {
@@ -364,14 +367,14 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
       D.Diag(clang::diag::warn_drv_unused_argument)
           << ArchArg->getAsString(Args);
     ArchName = StringRef(WaArch->getValue()).substr(7);
-    checkARMArchName(D, WaArch, Args, ArchName, CPUName,
-                     ExtensionFeatures, Triple);
+    checkARMArchName(D, WaArch, Args, ArchName, CPUName, ExtensionFeatures,
+                     Triple, ArchArgFPUID);
     // FIXME: Set Arch.
     D.Diag(clang::diag::warn_drv_unused_argument) << WaArch->getAsString(Args);
   } else if (ArchArg) {
     ArchName = ArchArg->getValue();
-    checkARMArchName(D, ArchArg, Args, ArchName, CPUName,
-                     ExtensionFeatures, Triple);
+    checkARMArchName(D, ArchArg, Args, ArchName, CPUName, ExtensionFeatures,
+                     Triple, ArchArgFPUID);
   }
 
   // Add CPU features for generic CPUs
@@ -390,8 +393,8 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
   }
 
   if (CPUArg)
-    checkARMCPUName(D, CPUArg, Args, CPUName, ArchName,
-                    ExtensionFeatures, Triple);
+    checkARMCPUName(D, CPUArg, Args, CPUName, ArchName, ExtensionFeatures,
+                    Triple, CPUArgFPUID);
   // Honor -mfpu=. ClangAs gives preference to -Wa,-mfpu=.
   unsigned FPUID = llvm::ARM::FK_INVALID;
   const Arg *FPUArg = Args.getLastArg(options::OPT_mfpu_EQ);
@@ -455,20 +458,26 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
       Features.push_back("+fullfp16");
   }
 
-  // Setting -msoft-float/-mfloat-abi=soft effectively disables the FPU (GCC
-  // ignores the -mfpu options in this case).
-  // Note that the ABI can also be set implicitly by the target selected.
+  // Setting -msoft-float/-mfloat-abi=soft, -mfpu=none, or adding +nofp to
+  // -march/-mcpu effectively disables the FPU (GCC ignores the -mfpu options in
+  // this case). Note that the ABI can also be set implicitly by the target
+  // selected.
   if (ABI == arm::FloatABI::Soft) {
     llvm::ARM::getFPUFeatures(llvm::ARM::FK_NONE, Features);
 
     // Disable all features relating to hardware FP, not already disabled by the
     // above call.
+    Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-bf16", "-mve",
+                                     "-mve.fp", "-fpregs"});
+  } else if (FPUID == llvm::ARM::FK_NONE ||
+             ArchArgFPUID == llvm::ARM::FK_NONE ||
+             CPUArgFPUID == llvm::ARM::FK_NONE) {
+    // -mfpu=none, -march=armvX+nofp or -mcpu=X+nofp is *very* similar to
+    // -mfloat-abi=soft, only that it should not disable MVE-I. They disable the
+    // FPU, but not the FPU registers, thus MVE-I, which depends only on the
+    // latter, is still supported.
     Features.insert(Features.end(),
-                    {"-dotprod", "-fp16fml", "-mve", "-mve.fp", "-fpregs"});
-  } else if (FPUID == llvm::ARM::FK_NONE) {
-    // -mfpu=none is *very* similar to -mfloat-abi=soft, only that it should not
-    // disable MVE-I.
-    Features.insert(Features.end(), {"-dotprod", "-fp16fml", "-mve.fp"});
+                    {"-dotprod", "-fp16fml", "-bf16", "-mve.fp"});
     if (!hasIntegerMVE(Features))
       Features.emplace_back("-fpregs");
   }

clang/test/CodeGen/arm-bf16-softfloat.c

@@ -1,4 +1,9 @@
-// RUN: not %clang -o %t.out -target arm-arm-eabi -march=armv8-a+bf16 -mfloat-abi=soft -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16 -mfloat-abi=soft -c %s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16 -mfpu=none -c %s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16+nofp -c %s -o %t 2>&1 | FileCheck %s
+// RUN: not %clang -target arm-arm-none-eabi -march=armv8-a+bf16+fp+nofp -c %s -o %t 2>&1 | FileCheck %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv8-a+bf16+fp -c %s -o %t
+// RUN: %clang -target arm-arm-none-eabi -march=armv8-a+bf16+nofp+fp -c %s -o %t
 
 // CHECK: error: __bf16 is not supported on this target
 extern __bf16 var;

clang/test/Driver/arm-nofp-disabled-features.c

@@ -0,0 +1,18 @@
+// RUN: %clang -target arm-arm-none-eabi -mfloat-abi=soft %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MFLOAT-ABI-SOFT
+// CHECK-MFLOAT-ABI-SOFT: "-target-feature" "-dotprod"
+// CHECK-MFLOAT-ABI-SOFT: "-target-feature" "-fp16fml"
+// CHECK-MFLOAT-ABI-SOFT: "-target-feature" "-bf16"
+// CHECK-MFLOAT-ABI-SOFT: "-target-feature" "-mve"
+// CHECK-MFLOAT-ABI-SOFT: "-target-feature" "-mve.fp"
+// CHECK-MFLOAT-ABI-SOFT: "-target-feature" "-fpregs"
+
+// RUN: %clang -target arm-arm-none-eabi -mfpu=none %s -### 2>&1 | FileCheck %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv8-a+nofp %s -### 2>&1 | FileCheck %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a35+nofp %s -### 2>&1 | FileCheck %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv8-a+nofp+nomve %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-NOMVE
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a35+nofp+nomve %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-NOMVE
+// CHECK: "-target-feature" "-dotprod"
+// CHECK: "-target-feature" "-fp16fml"
+// CHECK: "-target-feature" "-bf16"
+// CHECK: "-target-feature" "-mve.fp"
+// CHECK-NOMVE: "-target-feature" "-fpregs"

llvm/include/llvm/Support/ARMTargetParser.h

@@ -250,7 +250,8 @@ StringRef getSubArch(ArchKind AK);
 StringRef getArchExtName(uint64_t ArchExtKind);
 StringRef getArchExtFeature(StringRef ArchExt);
 bool appendArchExtFeatures(StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
-                           std::vector<StringRef> &Features);
+                           std::vector<StringRef> &Features,
+                           unsigned &ArgFPUKind);
 StringRef getHWDivName(uint64_t HWDivKind);
 
 // Information by Name

llvm/lib/Support/ARMTargetParser.cpp

@@ -490,9 +490,10 @@ static unsigned findDoublePrecisionFPU(unsigned InputFPUKind) {
   return ARM::FK_INVALID;
 }
 
-bool ARM::appendArchExtFeatures(
-  StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
-  std::vector<StringRef> &Features) {
+bool ARM::appendArchExtFeatures(StringRef CPU, ARM::ArchKind AK,
+                                StringRef ArchExt,
+                                std::vector<StringRef> &Features,
+                                unsigned &ArgFPUID) {
 
   size_t StartingNumFeatures = Features.size();
   const bool Negated = stripNegationPrefix(ArchExt);
@@ -527,6 +528,7 @@ bool ARM::appendArchExtFeatures(
     } else {
       FPUKind = getDefaultFPU(CPU, AK);
     }
+    ArgFPUID = FPUKind;
     return ARM::getFPUFeatures(FPUKind, Features);
   }
   return StartingNumFeatures != Features.size();

llvm/unittests/Support/TargetParserTest.cpp

@@ -668,9 +668,10 @@ static bool
 testArchExtDependency(const char *ArchExt,
                       const std::initializer_list<const char *> &Expected) {
   std::vector<StringRef> Features;
+  unsigned FPUID;
 
   if (!ARM::appendArchExtFeatures("", ARM::ArchKind::ARMV8_1MMainline, ArchExt,
-                                  Features))
+                                  Features, FPUID))
     return false;
 
   return llvm::all_of(Expected, [&](StringRef Ext) {