[clang] [Clang][ARM] Ensure FPU Features are parsed when targeting `cc1as` (PR #134612)

Martin Storsjö via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 10 05:24:56 PDT 2025


================
@@ -679,21 +679,17 @@ 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);
-    }
+    std::string CPU = arm::getARMTargetCPU(CPUName, ArchName, Triple);
+    bool Generic = CPU == "generic";
     if (Generic && (Triple.isOSWindows() || Triple.isOSDarwin()) &&
         getARMSubArchVersionNumber(Triple) >= 7) {
       FPUKind = llvm::ARM::parseFPU("neon");
----------------
mstorsjo wrote:

Those are somewhat still needed in order to keep things working for that patched case, yes.

It's indeed a bit odd to avoid breakage for an out-of-tree downstream patch that isn't present here, this was discussed in #122095, but there @DavidSpickett agreed that it seems good to be explicit about these things - just like for the Android case just above here.

The problem with adjusting the downstream patch is that it is for Debian/Ubuntu packaging, adjusting what is included in the armv7 baseline (for the purposes of Linux targets), but affecting how useful it is as a cross compiler for other targets. But for them to take extra effort into their Linux-targeting patch to accommodate for using it for cross compilation for other OSes feels like a big thing to request anyway (even though it is their patch which is breaking things).

> getARMCPUForArch already has some cpu overrides for different OS's.

Yeah, that would be possible as well. Windows does have one, and thanks to this patch it gets applied on both assembly and C. Darwin doesn't, but it should be possible to pick e.g. a `cortex-a8` as baseline there.

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


More information about the cfe-commits mailing list