[clang] [clang] [ARM] Explicitly enable NEON for Windows/Darwin targets (PR #122095)

Martin Storsjö via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 4 06:20:45 PDT 2025


================
@@ -659,13 +659,21 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
         CPUArgFPUKind != llvm::ARM::FK_INVALID ? CPUArgFPUKind : ArchArgFPUKind;
     (void)llvm::ARM::getFPUFeatures(FPUKind, Features);
   } else {
+    bool Generic = true;
     if (!ForAS) {
       std::string CPU = arm::getARMTargetCPU(CPUName, ArchName, Triple);
+      if (CPU != "generic")
+        Generic = false;
       llvm::ARM::ArchKind ArchKind =
           arm::getLLVMArchKindForARM(CPU, ArchName, Triple);
       FPUKind = llvm::ARM::getDefaultFPU(CPU, ArchKind);
       (void)llvm::ARM::getFPUFeatures(FPUKind, Features);
     }
+    if (Generic && (Triple.isOSWindows() || Triple.isOSDarwin()) &&
----------------
mstorsjo wrote:

> If you were to split out the ForAS change, would that be targeted enough to fix @anemet 's issue, or would that backport be just as impactful as https://github.com/llvm/llvm-project/pull/134366 ?

That PR is exactly the split out form of the targeted fix for @anemet's issue - but it does change whether FPU features are available in the assembler overall, so it is a bit of a potentially breaking change.

I did experiment with an even smaller change, something along these lines
```diff
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index c648fb66085c..5b1721583d05 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -659,11 +659,9 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
         CPUArgFPUKind != llvm::ARM::FK_INVALID ? CPUArgFPUKind : ArchArgFPUKind;
     (void)llvm::ARM::getFPUFeatures(FPUKind, Features);
   } else {
-    bool Generic = true;
+    std::string CPU = arm::getARMTargetCPU(CPUName, ArchName, Triple);
+    bool Generic = "generic";
     if (!ForAS) {
-      std::string CPU = arm::getARMTargetCPU(CPUName, ArchName, Triple);
-      if (CPU != "generic")
-        Generic = false;
       llvm::ARM::ArchKind ArchKind =
           arm::getLLVMArchKindForARM(CPU, ArchName, Triple);
       FPUKind = llvm::ARM::getDefaultFPU(CPU, ArchKind);
```

Unfortunately, this does break a bunch of the cases that this PR did attempt to fix. Things work fine as is in upstream llvm, but with the downstream Debian patch from https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/19/debian/patches/clang-arm-default-vfp3-on-armv7a.patch?ref_type=heads, things would still break again on Windows (and for armv7s+Darwin). This because Windows on ARM gets a default CPU of `cortex-a9` (not `generic`, and armv7s+Darwin gets `swift`), so we don't add the explicit NEON that we wanted.

https://github.com/llvm/llvm-project/pull/122095


More information about the cfe-commits mailing list