[Driver] Add defaults for Android ARM FPUs.

[danalbert]

Summary

Android mandates that devices have at least vfpv3-d16 until
Marshmallow and NEON after that. Still honor the user's decision, but
raise the defaults for Android targets.

[danalbert]

Related to this but something I was less sure we should do: Android no longer supports ARMv5. Should we make arm-linux-androideabi targets auto pull up to armv7 if there's no -march flag?

[srhines]

This LGTM, but we should wait to hear from Kristof before submitting.

[kristof.beyls]

In D53121#1261408, @srhines wrote:

This LGTM, but we should wait to hear from Kristof before submitting.

Seems fine to me too. I'd maybe just add an additional test case to verify that things still work as expected when users explicitly specify that they want to target a different FPU (e.g. "-mfpu=none").

[peter.smith]

It looks like we might be able to simplify a bit, and I think there probably could be some more test cases but no objections from me.

lib/Driver/ToolChains/Arch/ARM.cpp

360

Do you need to parse the arch version here? I would expect the -march=armv7 to be reflected in getARMSubArchVersionNumber(Triple)

If reduce this to ArchVersion = getARMSubArchVersionNumber(Triple) and add a test with -target arm-linux-androideabi21 -march=armv7 then everything still passes.

If I'm right you should be able to simplify this and perhaps roll it into the if (Triple.isAndroid() && ArchVersion >= 7) below. If I'm wrong can we add a test case that fails without the ArchVersion = llvm::ARM::parseArchVersion(ArchName) ?

It will also be worth adding tests for a generic target with -march

[danalbert]

lib/Driver/ToolChains/Arch/ARM.cpp

360

I had assumed that that wouldn't handle a case like -target arm-linux-androideabi21 -march=armv7-a where the -target argument specifies ARMv5 but -march rasies that to ARMv7, but you're right, the triple is updated to account for -march (which makes sense now that I think about it; that would lead to a lot of messy code all over if it didn't). Cleaned this up.

(I think I've also added the test you're asking for, but lmk if I misunderstood what you meant by "generic target")

test/Driver/arm-mfpu.c

410

In D53121#1261602, @kristof.beyls wrote:

Seems fine to me too. I'd maybe just add an additional test case to verify that things still work as expected when users explicitly specify that they want to target a different FPU (e.g. "-mfpu=none").

Is this test (and it's counterpart in CHECK-ARM-ANDROID-L-FP-NEON) not sufficient? It shows that -mfpu is honored regardless of the default. Is there something special about -mfpu=none that this doesn't exercise?

[danalbert]

Addressed review comments.

[kristof.beyls]

test/Driver/arm-mfpu.c

410

you're right - this test does what I was asking for. Apologies, I should've looked more closely - I hadn't picked up this mfpu isn't the default one...

[peter.smith]

Looks good to me. By generic target I just meant --target=arm-linux-androideabi21 with a -march to specify the revision, which you've got in the test.

lib/Driver/ToolChains/Arch/ARM.cpp

if (CPUArg)
           << CPUArg->getAsString(Args);
     CPUName = StringRef(WaCPU->getValue()).substr(6);
     checkARMCPUName(D, WaCPU, Args, CPUName, ArchName, Features, Triple);
   } else if (CPUArg) {
     CPUName = CPUArg->getValue();
     checkARMCPUName(D, CPUArg, Args, CPUName, ArchName, Features, Triple);
   }
 
+  unsigned ArchVersion;
+  if (ArchName.empty())
+      ArchVersion = getARMSubArchVersionNumber(Triple);
+  else
+      ArchVersion = llvm::ARM::parseArchVersion(ArchName);
+
   // Add CPU features for generic CPUs
   if (CPUName == "native") {
     llvm::StringMap<bool> HostFeatures;
     if (llvm::sys::getHostCPUFeatures(HostFeatures))
       for (auto &F : HostFeatures)
         Features.push_back(
             Args.MakeArgString((F.second ? "+" : "-") + F.first()));
   } else if (!CPUName.empty()) {
     DecodeARMFeaturesFromCPU(D, CPUName, Features);
   }
 
   // Honor -mfpu=. ClangAs gives preference to -Wa,-mfpu=.
   const Arg *FPUArg = Args.getLastArg(options::OPT_mfpu_EQ);
   if (WaFPU) {
     if (FPUArg)
       D.Diag(clang::diag::warn_drv_unused_argument)
           << FPUArg->getAsString(Args);
     getARMFPUFeatures(D, WaFPU, Args, StringRef(WaFPU->getValue()).substr(6),
                       Features);
   } else if (FPUArg) {
     getARMFPUFeatures(D, FPUArg, Args, FPUArg->getValue(), Features);
+  } else if (Triple.isAndroid() && ArchVersion >= 7) {
+    // Android mandates minimum FPU requirements based on OS version.
+    const char *AndroidFPU =
+        Triple.isAndroidVersionLT(23) ? "vfpv3-d16" : "neon";
+    if (!llvm::ARM::getFPUFeatures(llvm::ARM::parseFPU(AndroidFPU), Features))
+      D.Diag(clang::diag::err_drv_clang_unsupported)
+          << std::string("-mfpu=") + AndroidFPU;
   }
 
   // Honor -mhwdiv=. ClangAs gives preference to -Wa,-mhwdiv=.
   const Arg *HDivArg = Args.getLastArg(options::OPT_mhwdiv_EQ);
   if (WaHDiv) {
     if (HDivArg)
       D.Diag(clang::diag::warn_drv_unused_argument)
           << HDivArg->getAsString(Args);

test/Driver/arm-mfpu.c

 // CHECK-SOFT-ABI-FP: "-target-feature" "+soft-float"
 // CHECK-SOFT-ABI-FP: "-target-feature" "+soft-float-abi"
 // CHECK-SOFT-ABI-FP: "-target-feature" "-vfp2"
 // CHECK-SOFT-ABI-FP: "-target-feature" "-vfp3"
 // CHECK-SOFT-ABI-FP: "-target-feature" "-vfp4"
 // CHECK-SOFT-ABI-FP: "-target-feature" "-fp-armv8"
 // CHECK-SOFT-ABI-FP: "-target-feature" "-neon"
 // CHECK-SOFT-ABI-FP: "-target-feature" "-crypto"
+
+// RUN: %clang -target arm-linux-androideabi21 %s -### -c 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ARM5-ANDROID-FP-DEFAULT %s
+// CHECK-ARM5-ANDROID-FP-DEFAULT: "-target-feature" "+soft-float"
+// CHECK-ARM5-ANDROID-FP-DEFAULT: "-target-feature" "+soft-float-abi"
+// CHECK-ARM5-ANDROID-FP-DEFAULT-NOT: "-target-feature" "+d16"
+// CHECK-ARM5-ANDROID-FP-DEFAULT-NOT: "-target-feature" "+vfp3"
+// CHECK-ARM5-ANDROID-FP-DEFAULT-NOT: "-target-feature" "+vfp4"
+// CHECK-ARM5-ANDROID-FP-DEFAULT-NOT: "-target-feature" "+fp-armv8"
+// CHECK-ARM5-ANDROID-FP-DEFAULT-NOT: "-target-feature" "+neon"
+// CHECK-ARM5-ANDROID-FP-DEFAULT-NOT: "-target-feature" "+crypto"
+
+// RUN: %clang -target armv7-linux-androideabi21 %s -### -c 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ARM-ANDROID-L-FP-DEFAULT %s
+// CHECK-ARM-ANDROID-L-FP-DEFAULT-NOT: "-target-feature" "+soft-float"
+// CHECK-ARM-ANDROID-L-FP-DEFAULT: "-target-feature" "+soft-float-abi"
+// CHECK-ARM-ANDROID-L-FP-DEFAULT: "-target-feature" "+d16"
+// CHECK-ARM-ANDROID-L-FP-DEFAULT: "-target-feature" "+vfp3"
+// CHECK-ARM-ANDROID-L-FP-DEFAULT-NOT: "-target-feature" "+vfp4"
+// CHECK-ARM-ANDROID-L-FP-DEFAULT-NOT: "-target-feature" "+fp-armv8"
+// CHECK-ARM-ANDROID-L-FP-DEFAULT-NOT: "-target-feature" "+neon"
+// CHECK-ARM-ANDROID-L-FP-DEFAULT-NOT: "-target-feature" "+crypto"
+
+// RUN: %clang -target armv7-linux-androideabi21 -mfpu=neon %s -### -c 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ARM-ANDROID-L-FP-NEON %s
+// CHECK-ARM-ANDROID-L-FP-NEON-NOT: "-target-feature" "+soft-float"
+// CHECK-ARM-ANDROID-L-FP-NEON: "-target-feature" "+soft-float-abi"
+// CHECK-ARM-ANDROID-L-FP-NEON: "-target-feature" "+vfp3"
+// CHECK-ARM-ANDROID-L-FP-NEON-NOT: "-target-feature" "+vfp4"
+// CHECK-ARM-ANDROID-L-FP-NEON-NOT: "-target-feature" "+fp-armv8"
+// CHECK-ARM-ANDROID-L-FP-NEON: "-target-feature" "+neon"
+// CHECK-ARM-ANDROID-L-FP-NEON-NOT: "-target-feature" "+crypto"
+
+// RUN: %clang -target armv7-linux-androideabi23 %s -### -c 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ARM-ANDROID-M-FP-DEFAULT %s
+// CHECK-ARM-ANDROID-M-FP-DEFAULT-NOT: "-target-feature" "+soft-float"
+// CHECK-ARM-ANDROID-M-FP-DEFAULT: "-target-feature" "+soft-float-abi"
+// CHECK-ARM-ANDROID-M-FP-DEFAULT: "-target-feature" "+vfp3"
+// CHECK-ARM-ANDROID-M-FP-DEFAULT-NOT: "-target-feature" "+vfp4"
+// CHECK-ARM-ANDROID-M-FP-DEFAULT-NOT: "-target-feature" "+fp-armv8"
+// CHECK-ARM-ANDROID-M-FP-DEFAULT: "-target-feature" "+neon"
+// CHECK-ARM-ANDROID-M-FP-DEFAULT-NOT: "-target-feature" "+crypto"
+
+// RUN: %clang -target armv7-linux-androideabi23 %s -mfpu=vfp3-d16 -### -c 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ARM-ANDROID-M-FP-D16 %s
+// CHECK-ARM-ANDROID-M-FP-D16-NOT: "-target-feature" "+soft-float"
+// CHECK-ARM-ANDROID-M-FP-D16: "-target-feature" "+soft-float-abi"
+// CHECK-ARM-ANDROID-M-FP-D16: "-target-feature" "+d16"
+// CHECK-ARM-ANDROID-M-FP-D16: "-target-feature" "+vfp3"
+// CHECK-ARM-ANDROID-M-FP-D16-NOT: "-target-feature" "+vfp4"
+// CHECK-ARM-ANDROID-M-FP-D16-NOT: "-target-feature" "+fp-armv8"
+// CHECK-ARM-ANDROID-M-FP-D16-NOT: "-target-feature" "+neon"
+// CHECK-ARM-ANDROID-M-FP-D16-NOT: "-target-feature" "+crypto"