[ARM] Use TargetParser to determine FPU subtarget features

[john.brawn]

Summary

The main effect of this is to fix anomalies where certain -mfpu options didn't disable everything that they should causing strange behaviour when combined with -mcpu or -march values that themselves enabled fpu subtarget features, e.g. -mfpu=fpv5-dp-d16 with -march=armv7em previously behaved the same as -mfpu=fpv5-sp-d16 due to fp-only-sp not being disabled.

Invalid -mfpu options now also give an error, which is consistent with the handling of the .fpu directive.

[jmolloy]

Hi John,

This looks good to me. This is a nice cleanup!

Cheers,

James

[rengolin]

Once D10237 is good, we'll look into this one.

Tests look ok, too.

cheers,
--renato

lib/Driver/Tools.cpp

545 ↗

(On Diff #27107)

getFPUFeatures should probably go somewhere other than ARMTargetParser...

[rengolin]

We're not moving that method from the parser now, so LGTM. Thanks!

cfe/trunk/lib/Driver/Tools.cpp

static void getARMHWDivFeatures(const Driver &D, const Arg *A,
   } else if (HWDiv == "none") {
     Features.push_back("-hwdiv-arm");
     Features.push_back("-hwdiv");
   } else
     D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
 }
 
 // Handle -mfpu=.
-//
-// FIXME: Centralize feature selection, defaulting shouldn't be also in the
-// frontend target.
 static void getARMFPUFeatures(const Driver &D, const Arg *A,
                               const ArgList &Args,
                               std::vector<const char *> &Features) {
   StringRef FPU = A->getValue();
-
-  // FIXME: Why does "none" disable more than "invalid"?
-  if (FPU == "none") {
-    Features.push_back("-vfp2");
-    Features.push_back("-vfp3");
-    Features.push_back("-vfp4");
-    Features.push_back("-fp-armv8");
-    Features.push_back("-crypto");
-    Features.push_back("-neon");
-    return;
-  }
-
-  // FIXME: Make sure we differentiate sp-only.
-  if (FPU.find("-sp-") != StringRef::npos) {
-    Features.push_back("+fp-only-sp");
-  }
-
-  // All other FPU types, valid or invalid.
-  switch(llvm::ARMTargetParser::parseFPU(FPU)) {
-  case llvm::ARM::FK_INVALID:
-  case llvm::ARM::FK_SOFTVFP:
-    Features.push_back("-vfp2");
-    Features.push_back("-vfp3");
-    Features.push_back("-neon");
-    break;
-  case llvm::ARM::FK_VFP:
-  case llvm::ARM::FK_VFPV2:
-    Features.push_back("+vfp2");
-    Features.push_back("-neon");
-    break;
-  case llvm::ARM::FK_VFPV3_D16:
-    Features.push_back("+d16");
-    // fall-through
-  case llvm::ARM::FK_VFPV3:
-    Features.push_back("+vfp3");
-    Features.push_back("-neon");
-    break;
-  case llvm::ARM::FK_VFPV4_D16:
-    Features.push_back("+d16");
-    // fall-through
-  case llvm::ARM::FK_VFPV4:
-    Features.push_back("+vfp4");
-    Features.push_back("-neon");
-    break;
-  case llvm::ARM::FK_FPV5_D16:
-    Features.push_back("+d16");
-    // fall-through
-  case llvm::ARM::FK_FP_ARMV8:
-    Features.push_back("+fp-armv8");
-    Features.push_back("-neon");
-    Features.push_back("-crypto");
-    break;
-  case llvm::ARM::FK_NEON_FP_ARMV8:
-    Features.push_back("+fp-armv8");
-    Features.push_back("+neon");
-    Features.push_back("-crypto");
-    break;
-  case llvm::ARM::FK_CRYPTO_NEON_FP_ARMV8:
-    Features.push_back("+fp-armv8");
-    Features.push_back("+neon");
-    Features.push_back("+crypto");
-    break;
-  case llvm::ARM::FK_NEON:
-    Features.push_back("+neon");
-    break;
-  case llvm::ARM::FK_NEON_VFPV4:
-    Features.push_back("+neon");
-    Features.push_back("+vfp4");
-    break;
-  default:
+  unsigned FPUID = llvm::ARMTargetParser::parseFPU(FPU);
+  if (!llvm::ARMTargetParser::getFPUFeatures(FPUID, Features))
     D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
-  }
-}
 
 static int getARMSubArchVersionNumber(const llvm::Triple &Triple) {
   llvm::StringRef Arch = Triple.getArchName();
   return llvm::ARMTargetParser::parseArchVersion(Arch);
 }
 
 static bool isARMMProfile(const llvm::Triple &Triple) {
   llvm::StringRef Arch = Triple.getArchName();

cfe/trunk/test/Driver/arm-mfpu.c

 // RUN: %clang -target arm-linux-eabi -mfpu=fpa %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPA %s
 // RUN: %clang -target arm-linux-eabi -mfpu=fpe2 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPA %s
 // RUN: %clang -target arm-linux-eabi -mfpu=fpe3 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPA %s
 // RUN: %clang -target arm-linux-eabi -mfpu=maverick %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPA %s
-// CHECK-FPA: "-target-feature" "-vfp2"
-// CHECK-FPA: "-target-feature" "-vfp3"
-// CHECK-FPA: "-target-feature" "-neon"
+// CHECK-FPA: error: {{.*}} does not support '-mfpu={{fpa|fpe|fpe2|fpe3|maverick}}'
 
 // RUN: %clang -target arm-linux-eabi -mfpu=vfp %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-VFP %s
 // CHECK-VFP: "-target-feature" "+vfp2"
+// CHECK-VFP: "-target-feature" "-vfp3"
+// CHECK-VFP: "-target-feature" "-vfp4"
+// CHECK-VFP: "-target-feature" "-fp-armv8"
 // CHECK-VFP: "-target-feature" "-neon"
 
 // RUN: %clang -target arm-linux-eabi -mfpu=vfp3 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-VFP3 %s
 // RUN: %clang -target arm-linux-eabi -mfpu=vfpv3 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-VFP3 %s
 // CHECK-VFP3: "-target-feature" "+vfp3"
+// CHECK-VFP3: "-target-feature" "-vfp4"
+// CHECK-VFP3: "-target-feature" "-fp-armv8"
 // CHECK-VFP3: "-target-feature" "-neon"
 
 // RUN: %clang -target arm-linux-eabi -mfpu=vfp3-d16 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-VFP3-D16 %s
 // RUN: %clang -target arm-linux-eabi -mfpu=vfpv3-d16 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-VFP3-D16 %s
+// CHECK-VFP3-D16: "-target-feature" "-fp-only-sp"
 // CHECK-VFP3-D16: "-target-feature" "+d16"
 // CHECK-VFP3-D16: "-target-feature" "+vfp3"
+// CHECK-VFP3-D16: "-target-feature" "-vfp4"
+// CHECK-VFP3-D16: "-target-feature" "-fp-armv8"
 // CHECK-VFP3-D16: "-target-feature" "-neon"
 
 // RUN: %clang -target arm-linux-eabi -mfpu=vfp4 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-VFP4 %s
 // RUN: %clang -target arm-linux-eabi -mfpu=vfpv4 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-VFP4 %s
 // CHECK-VFP4: "-target-feature" "+vfp4"
+// CHECK-VFP4: "-target-feature" "-fp-armv8"
 // CHECK-VFP4: "-target-feature" "-neon"
 
 // RUN: %clang -target arm-linux-eabi -mfpu=vfp4-d16 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-VFP4-D16 %s
 // RUN: %clang -target arm-linux-eabi -mfpu=vfpv4-d16 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-VFP4-D16 %s
+// CHECK-VFP4-D16: "-target-feature" "-fp-only-sp"
 // CHECK-VFP4-D16: "-target-feature" "+d16"
 // CHECK-VFP4-D16: "-target-feature" "+vfp4"
+// CHECK-VFP4-D16: "-target-feature" "-fp-armv8"
 // CHECK-VFP4-D16: "-target-feature" "-neon"
 
 // RUN: %clang -target arm-linux-eabi -mfpu=fp4-sp-d16 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FP4-SP-D16 %s
 // RUN: %clang -target arm-linux-eabi -mfpu=fpv4-sp-d16 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FP4-SP-D16 %s
 // CHECK-FP4-SP-D16: "-target-feature" "+fp-only-sp"
 // CHECK-FP4-SP-D16: "-target-feature" "+d16"
 // CHECK-FP4-SP-D16: "-target-feature" "+vfp4"
+// CHECK-FP4-SP-D16: "-target-feature" "-fp-armv8"
 // CHECK-FP4-SP-D16: "-target-feature" "-neon"
 
 // RUN: %clang -target arm-linux-eabi -mfpu=fp5-sp-d16 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FP5-SP-D16 %s
 // RUN: %clang -target arm-linux-eabi -mfpu=fpv5-sp-d16 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FP5-SP-D16 %s
 // CHECK-FP5-SP-D16: "-target-feature" "+fp-only-sp"
 // CHECK-FP5-SP-D16: "-target-feature" "+d16"
 // CHECK-FP5-SP-D16: "-target-feature" "+fp-armv8"
 // CHECK-FP5-SP-D16: "-target-feature" "-neon"
 // CHECK-FP5-SP-D16: "-target-feature" "-crypto"
 
 // RUN: %clang -target arm-linux-eabi -mfpu=fp5-dp-d16 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FP5-DP-D16 %s
 // RUN: %clang -target arm-linux-eabi -mfpu=fpv5-dp-d16 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FP5-DP-D16 %s
+// CHECK-FP5-DP-D16: "-target-feature" "-fp-only-sp"
 // CHECK-FP5-DP-D16: "-target-feature" "+d16"
 // CHECK-FP5-DP-D16: "-target-feature" "+fp-armv8"
 // CHECK-FP5-DP-D16: "-target-feature" "-neon"
 // CHECK-FP5-DP-D16: "-target-feature" "-crypto"
 
 // RUN: %clang -target arm-linux-eabi -mfpu=neon %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NEON %s
 // CHECK-NEON: "-target-feature" "+neon"
 
 // RUN: %clang -target arm-linux-eabi -mfpu=neon-vfpv3 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NEON-VFPV3 %s
 // CHECK-NEON-VFPV3: "-target-feature" "+neon"
 
 // RUN: %clang -target arm-linux-eabi -mfpu=neon-vfpv4 %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NEON-VFPV4 %s
-// CHECK-NEON-VFPV4: "-target-feature" "+neon"
 // CHECK-NEON-VFPV4: "-target-feature" "+vfp4"
+// CHECK-NEON-VFPV4: "-target-feature" "+neon"
 
 // RUN: %clang -target arm-linux-eabi -msoft-float %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-SOFT-FLOAT %s
 // CHECK-SOFT-FLOAT: "-target-feature" "-neon"
 
 // RUN: %clang -target armv8 %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-ARMV8-DEFAULT-SOFT-FP %s
 // CHECK-ARMV8-DEFAULT-SOFT-FP: "-target-feature" "-neon"

 // RUN:   | FileCheck --check-prefix=CHECK-NEON-FP-ARMV8 %s
 // CHECK-NEON-FP-ARMV8: "-target-feature" "+fp-armv8"
 // CHECK-NEON-FP-ARMV8: "-target-feature" "+neon"
 // CHECK-NEON-FP-ARMV8: "-target-feature" "-crypto"
 
 // RUN: %clang -target armv8-linux-gnueabihf -mfpu=crypto-neon-fp-armv8 %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-CRYPTO-NEON-FP-ARMV8 %s
 // CHECK-CRYPTO-NEON-FP-ARMV8: "-target-feature" "+fp-armv8"
-// CHECK-CRYPTO-NEON-FP-ARMV8: "-target-feature" "+neon"
 // CHECK-CRYPTO-NEON-FP-ARMV8: "-target-feature" "+crypto"
 
 // RUN: %clang -target armv8-linux-gnueabi -mfpu=none %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FP %s
+// CHECK-NO-FP: "-target-feature" "-fp-only-sp"
+// CHECK-NO-FP: "-target-feature" "-d16"
 // CHECK-NO-FP: "-target-feature" "-vfp2"
 // CHECK-NO-FP: "-target-feature" "-vfp3"
 // CHECK-NO-FP: "-target-feature" "-vfp4"
 // CHECK-NO-FP: "-target-feature" "-fp-armv8"
-// CHECK-NO-FP: "-target-feature" "-crypto"
 // CHECK-NO-FP: "-target-feature" "-neon"
+// CHECK-NO-FP: "-target-feature" "-crypto"
 
 // RUN: %clang -target arm-linux-gnueabihf %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-HF %s
 // CHECK-HF: "-target-cpu" "arm1176jzf-s"
 
 // RUN: %clang -target armv7-apple-darwin -x assembler %s -### -c 2>&1 \
 // RUN:   | FileCheck --check-prefix=ASM %s
 // ASM-NOT: -target-feature

cfe/trunk/test/Preprocessor/arm-target-features.c

 // DEFAULTFPU-A5:#define __ARM_NEON__ 1
 // DEFAULTFPU-A5:#define __ARM_VFPV4__ 1
 
 // RUN: %clang -target armv7-none-linux-gnueabi -mcpu=cortex-a5 -mfpu=none -x c -E -dM %s -o - | FileCheck --check-prefix=FPUNONE-A5 %s
 // RUN: %clang -target armv7-none-linux-gnueabi -mthumb -mcpu=cortex-a5 -mfpu=none -x c -E -dM %s -o - | FileCheck --check-prefix=FPUNONE-A5 %s
 // FPUNONE-A5-NOT:#define __ARM_NEON__ 1
 // FPUNONE-A5-NOT:#define __ARM_VFPV4__ 1
 
-// RUN: %clang -target armv7-none-linux-gnueabi -mcpu=cortex-a5 -mfpu=vfp3-d16 -x c -E -dM %s -o - | FileCheck --check-prefix=NONEON-A5 %s
-// RUN: %clang -target armv7-none-linux-gnueabi -mthumb -mcpu=cortex-a5 -mfpu=vfp3-d16 -x c -E -dM %s -o - | FileCheck --check-prefix=NONEON-A5 %s
+// RUN: %clang -target armv7-none-linux-gnueabi -mcpu=cortex-a5 -mfpu=vfp4-d16 -x c -E -dM %s -o - | FileCheck --check-prefix=NONEON-A5 %s
+// RUN: %clang -target armv7-none-linux-gnueabi -mthumb -mcpu=cortex-a5 -mfpu=vfp4-d16 -x c -E -dM %s -o - | FileCheck --check-prefix=NONEON-A5 %s
 // NONEON-A5-NOT:#define __ARM_NEON__ 1
 // NONEON-A5:#define __ARM_VFPV4__ 1
 
 // FIXME: add check for further predefines
 // Test whether predefines are as expected when targeting ep9312.
 // RUN: %clang -target armv4t -mcpu=ep9312 -x c -E -dM %s -o - | FileCheck --check-prefix=A4T %s
 // A4T-NOT:#define __ARM_FEATURE_DSP