[Driver][ARM] Change Android's NEON FPU hardcoding to "== 7" as it pessimizes future ArmV8 code

[Sharjeel-Khan]

Android hardcoded NEON FPU for ARM version ">=" 7. This hardcoding was pessimizing ARMv8 code as it was locking it to NEON FPU instead of something more powerful.

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Sharjeel Khan (Sharjeel-Khan)

Changes

Android hardcoded NEON FPU for armv7 that is now causing issues with 32-bit builds for armv8 on Android.

---

Full diff: https://github.com/llvm/llvm-project/pull/122969.diff

2 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Arch/ARM.cpp (+1-1)
• (modified) clang/test/Driver/arm-mfpu.c (+19)

diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index b8181ce6dc012a..2fb16d2e41320f 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -647,7 +647,7 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
     (void)getARMFPUFeatures(D, WaFPU->first, Args, WaFPU->second, Features);
   } else if (FPUArg) {
     FPUKind = getARMFPUFeatures(D, FPUArg, Args, FPUArg->getValue(), Features);
-  } else if (Triple.isAndroid() && getARMSubArchVersionNumber(Triple) >= 7) {
+  } else if (Triple.isAndroid() && getARMSubArchVersionNumber(Triple) == 7) {
     const char *AndroidFPU = "neon";
     FPUKind = llvm::ARM::parseFPU(AndroidFPU);
     if (!llvm::ARM::getFPUFeatures(FPUKind, Features))
diff --git a/clang/test/Driver/arm-mfpu.c b/clang/test/Driver/arm-mfpu.c
index 1b174be388124d..babfa16741ad70 100644
--- a/clang/test/Driver/arm-mfpu.c
+++ b/clang/test/Driver/arm-mfpu.c
@@ -409,6 +409,25 @@
 // CHECK-ARM7-ANDROID-FP-DEFAULT-NOT: "-target-feature" "+sha2"
 // CHECK-ARM7-ANDROID-FP-DEFAULT-NOT: "-target-feature" "+aes"
 
+// RUN: %clang -target armv8-linux-androideabi21 %s -### -c 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ARM8-ANDROID-FP-DEFAULT %s
+// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+soft-float-abi"
+// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+vfp3"
+// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+vfp4"
+// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+fp-armv8"
+// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+aes"
+// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+sha2"
+// CHECK-ARM8-ANDROID-FP-DEFAULT-NOT: "-target-feature" "+neon"
+
+// RUN: %clang -target armv8-linux-android %s -### -c 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ARM8-ANDROID-DEFAULT %s
+// CHECK-ARM8-ANDROID-DEFAULT-DAG: "-target-feature" "+vfp3"
+// CHECK-ARM8-ANDROID-DEFAULT-DAG: "-target-feature" "+vfp4"
+// CHECK-ARM8-ANDROID-DEFAULT-DAG: "-target-feature" "+fp-armv8"
+// CHECK-ARM8-ANDROID-DEFAULT-DAG: "-target-feature" "+aes"
+// CHECK-ARM8-ANDROID-DEFAULT-DAG: "-target-feature" "+sha2"
+// CHECK-ARM8-ANDROID-DEFAULT-NOT: "-target-feature" "+neon"
+
 // RUN: %clang -target armv7-linux-androideabi21 %s -mfpu=vfp3-d16 -### -c 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-ARM7-ANDROID-FP-D16 %s
 // CHECK-ARM7-ANDROID-FP-D16-NOT: "-target-feature" "+soft-float"

[stephenhines]

The only thing I might add is a bit more context in the commit message that by accidentally using ">= 7" previously, we were pessimizing ARMv8 code in the future (locking it into just NEON FPU, instead of something more powerful).

[DavidSpickett]

Yeah, my first impression from the commit message was that you removed the check entirely.

[DavidSpickett]

Make sure to update the PR Description as this becomes the commit message when the squash and merge happens.

(yes, this is confusing but that's how llvm is set up)

But what I read makes sense thank you for rephrasing the commit message.

[Sharjeel-Khan]

Thanks, I updated the PR description. Do you mind merging it @DavidSpickett? I still do not have commit access.