[clang][Arm] Fix handling of -Wa,-march=

This fixes Bugzilla #48894 for Arm, where it
was reported that -Wa,-march was not being handled
by the integrated assembler.

This was previously fixed for -Wa,-mthumb by
parsing the argument in ToolChain::ComputeLLVMTriple
instead of CollectArgsForIntegratedAssembler.
It has to be done in the former because the Triple
is read only by the time we get to the latter.

Previously only mcpu would work via -Wa but only because
"-target-cpu" is it's own option to cc1, which we were
able to modify. Target architecture is part of "-target-triple".

This change applies the same workaround to -march and cleans up
handling of -Wa,-mcpu at the same time. There were some
places where we were not using the last instance of an argument.

The existing -Wa,-mthumb code was doing this correctly,
so I've just added tests to confirm that.

Now the same rules will apply to -Wa,-march/-mcpu as would
if you just passed them to the compiler:
* -Wa/-Xassembler options only apply to assembly files.
* Architecture derived from mcpu beats any march options.
* When there are multiple mcpu or multiple march, the last
  one wins.
* If there is a compiler option and an assembler option of
  the same type, we prefer the one that fits the input type.
* If there is an applicable mcpu option but it is overruled
  by an march, the cpu value is still used for the "-target-cpu"
  cc1 option.

Reviewed By: nickdesaulniers

Differential Revision: https://reviews.llvm.org/D95872

clang/lib/Driver/ToolChain.cpp

@@ -786,15 +786,26 @@ std::string ToolChain::ComputeLLVMTriple(const ArgList &Args,
     else {
       // Ideally we would check for these flags in
       // CollectArgsForIntegratedAssembler but we can't change the ArchName at
-      // that point. There is no assembler equivalent of -mno-thumb, -marm, or
-      // -mno-arm.
+      // that point.
+      llvm::StringRef WaMArch, WaMCPU;
       for (const auto *A :
            Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler)) {
         for (StringRef Value : A->getValues()) {
+          // There is no assembler equivalent of -mno-thumb, -marm, or -mno-arm.
           if (Value == "-mthumb")
             IsThumb = true;
+          else if (Value.startswith("-march="))
+            WaMArch = Value.substr(7);
+          else if (Value.startswith("-mcpu="))
+            WaMCPU = Value.substr(6);
         }
       }
+
+      if (WaMCPU.size() || WaMArch.size()) {
+        // The way this works means that we prefer -Wa,-mcpu's architecture
+        // over -Wa,-march. Which matches the compiler behaviour.
+        Suffix = tools::arm::getLLVMArchSuffixForARM(WaMCPU, WaMArch, Triple);
+      }
     }
     // Assembly files should start in ARM mode, unless arch is M-profile, or
     // -mthumb has been passed explicitly to the assembler. Windows is always

clang/lib/Driver/ToolChains/Arch/ARM.cpp

@@ -50,11 +50,14 @@ void arm::getARMArchCPUFromArgs(const ArgList &Args, llvm::StringRef &Arch,
 
   for (const Arg *A :
        Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler)) {
-    StringRef Value = A->getValue();
-    if (Value.startswith("-mcpu="))
-      CPU = Value.substr(6);
-    if (Value.startswith("-march="))
-      Arch = Value.substr(7);
+    // Use getValues because -Wa can have multiple arguments
+    // e.g. -Wa,-mcpu=foo,-mcpu=bar
+    for (StringRef Value : A->getValues()) {
+      if (Value.startswith("-mcpu="))
+        CPU = Value.substr(6);
+      if (Value.startswith("-march="))
+        Arch = Value.substr(7);
+    }
   }
 }
 
@@ -290,8 +293,8 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
       Args.hasArg(options::OPT_mkernel, options::OPT_fapple_kext);
   arm::FloatABI ABI = arm::getARMFloatABI(D, Triple, Args);
   arm::ReadTPMode ThreadPointer = arm::getReadTPMode(D, Args);
-  const Arg *WaCPU = nullptr, *WaFPU = nullptr;
-  const Arg *WaHDiv = nullptr, *WaArch = nullptr;
+  llvm::Optional<std::pair<const Arg *, StringRef>> WaCPU, WaFPU, WaHDiv,
+      WaArch;
 
   // This vector will accumulate features from the architecture
   // extension suffixes on -mcpu and -march (e.g. the 'bar' in
@@ -325,15 +328,18 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
     // to the assembler correctly.
     for (const Arg *A :
          Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler)) {
-      StringRef Value = A->getValue();
-      if (Value.startswith("-mfpu=")) {
-        WaFPU = A;
-      } else if (Value.startswith("-mcpu=")) {
-        WaCPU = A;
-      } else if (Value.startswith("-mhwdiv=")) {
-        WaHDiv = A;
-      } else if (Value.startswith("-march=")) {
-        WaArch = A;
+      // We use getValues here because you can have many options per -Wa
+      // We will keep the last one we find for each of these
+      for (StringRef Value : A->getValues()) {
+        if (Value.startswith("-mfpu=")) {
+          WaFPU = std::make_pair(A, Value.substr(6));
+        } else if (Value.startswith("-mcpu=")) {
+          WaCPU = std::make_pair(A, Value.substr(6));
+        } else if (Value.startswith("-mhwdiv=")) {
+          WaHDiv = std::make_pair(A, Value.substr(8));
+        } else if (Value.startswith("-march=")) {
+          WaArch = std::make_pair(A, Value.substr(7));
+        }
       }
     }
   }
@@ -353,8 +359,8 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
     if (CPUArg)
       D.Diag(clang::diag::warn_drv_unused_argument)
           << CPUArg->getAsString(Args);
-    CPUName = StringRef(WaCPU->getValue()).substr(6);
-    CPUArg = WaCPU;
+    CPUName = WaCPU->second;
+    CPUArg = WaCPU->first;
   } else if (CPUArg)
     CPUName = CPUArg->getValue();
 
@@ -363,11 +369,12 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
     if (ArchArg)
       D.Diag(clang::diag::warn_drv_unused_argument)
           << ArchArg->getAsString(Args);
-    ArchName = StringRef(WaArch->getValue()).substr(7);
-    checkARMArchName(D, WaArch, Args, ArchName, CPUName, ExtensionFeatures,
-                     Triple, ArchArgFPUID);
-    // FIXME: Set Arch.
-    D.Diag(clang::diag::warn_drv_unused_argument) << WaArch->getAsString(Args);
+    ArchName = WaArch->second;
+    // This will set any features after the base architecture.
+    checkARMArchName(D, WaArch->first, Args, ArchName, CPUName,
+                     ExtensionFeatures, Triple, ArchArgFPUID);
+    // The base architecture was handled in ToolChain::ComputeLLVMTriple because
+    // triple is read only by this point.
   } else if (ArchArg) {
     ArchName = ArchArg->getValue();
     checkARMArchName(D, ArchArg, Args, ArchName, CPUName, ExtensionFeatures,
@@ -399,8 +406,7 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
     if (FPUArg)
       D.Diag(clang::diag::warn_drv_unused_argument)
           << FPUArg->getAsString(Args);
-    (void)getARMFPUFeatures(D, WaFPU, Args, StringRef(WaFPU->getValue()).substr(6),
-                            Features);
+    (void)getARMFPUFeatures(D, WaFPU->first, Args, WaFPU->second, Features);
   } else if (FPUArg) {
     FPUID = getARMFPUFeatures(D, FPUArg, Args, FPUArg->getValue(), Features);
   } else if (Triple.isAndroid() && getARMSubArchVersionNumber(Triple) >= 7) {
@@ -423,8 +429,7 @@ void arm::getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
     if (HDivArg)
       D.Diag(clang::diag::warn_drv_unused_argument)
           << HDivArg->getAsString(Args);
-    getARMHWDivFeatures(D, WaHDiv, Args,
-                        StringRef(WaHDiv->getValue()).substr(8), Features);
+    getARMHWDivFeatures(D, WaHDiv->first, Args, WaHDiv->second, Features);
   } else if (HDivArg)
     getARMHWDivFeatures(D, HDivArg, Args, HDivArg->getValue(), Features);
 

clang/test/Driver/arm-target-as-march-mcpu.s

@@ -0,0 +1,104 @@
+/// These tests make sure that options passed to the assembler
+/// via -Wa or -Xassembler are applied correctly to assembler inputs.
+/// Also we check that the same priority rules apply to compiler and
+/// assembler options.
+///
+/// Note that the cortex-a8 is armv7-a, the cortex-a32 is armv8-a
+/// and clang's default Arm architecture is armv4t.
+
+/// Sanity check how the options behave when passed to the compiler
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv7-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv7-a+crc %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,EXT-CRC %s
+
+/// -Wa/-Xassembler doesn't apply to non assembly files
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-march=armv7-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TRIPLE-ARMV4 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -march=armv7-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TRIPLE-ARMV4 %s
+
+/// -Wa/-Xassembler does apply to assembler input
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-march=armv7-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-march=armv7-a+crc %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,EXT-CRC %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -march=armv7-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -march=armv7-a+crc %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,EXT-CRC %s
+
+/// Check that arch name is still canonicalised
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-march=armv7a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -march=armv7 %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+
+/// march to compiler and assembler, we choose the one suited to the input file type
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv8-a -Wa,-march=armv7a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv7-a -Wa,-march=armv8-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TRIPLE-ARMV7 %s
+
+/// mcpu to compiler and march to assembler, we use the assembler's architecture for assembly files.
+/// We use the target CPU for both.
+// RUN: %clang -target arm-linux-gnueabi -### -c -mcpu=cortex-a8 -Wa,-march=armv8a %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV8,CPU-A8 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -mcpu=cortex-a8 -Wa,-march=armv8-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+
+/// march to compiler and mcpu to assembler, we use the one that matches the file type
+/// (again both get the target-cpu option either way)
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv8a -Wa,-mcpu=cortex-a8 %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv8a -Wa,-mcpu=cortex-a8 \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TRIPLE-ARMV8 %s
+
+/// march and mcpu to the compiler, mcpu wins
+// RUN: %clang -target arm-linux-gnueabi -### -c -mcpu=cortex-a8 -march=armv8-a %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+/// not dependent on order
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv8-a -mcpu=cortex-a8 %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+/// or file type
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv8a -mcpu=cortex-a8 \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+
+/// If we pass mcpu and march to the assembler then mcpu's arch wins
+/// (matches the compiler behaviour)
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-mcpu=cortex-a8 -Wa,-march=armv8-a %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-mcpu=cortex-a8,-march=armv8-a %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -march=armv8-a -Xassembler -mcpu=cortex-a8 \
+// RUN: %s 2>&1 | FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+
+/// Last mcpu to assembler wins
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-mcpu=cortex-a32,-mcpu=cortex-a8 %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-mcpu=cortex-a32 -Wa,-mcpu=cortex-a8 %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 --check-prefix=CPU-A8 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -mcpu=cortex-a32 -Xassembler -mcpu=cortex-a8 \
+// RUN: %s 2>&1 | FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+
+/// Last mcpu to compiler wins
+// RUN: %clang -target arm-linux-gnueabi -### -c -mcpu=cortex-a32 -mcpu=cortex-a8 %s 2>&1 | \
+// RUN: FileCheck --check-prefixes=TRIPLE-ARMV7,CPU-A8 %s
+
+/// Last march to assembler wins
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-march=armv8-a,-march=armv7-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Wa,-march=armv8-a -Wa,-march=armv7-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+// RUN: %clang -target arm-linux-gnueabi -### -c -Xassembler -march=armv8-a -Xassembler -march=armv7-a \
+// RUN: %s 2>&1 | FileCheck --check-prefix=TRIPLE-ARMV7 %s
+
+/// Last march to compiler wins
+// RUN: %clang -target arm-linux-gnueabi -### -c -march=armv8-a -march=armv7-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TRIPLE-ARMV7 %s
+
+// TRIPLE-ARMV4: "-triple" "armv4t-unknown-linux-gnueabi"
+// TRIPLE-ARMV7: "-triple" "armv7-unknown-linux-gnueabi"
+// TRIPLE-ARMV8: "-triple" "armv8-unknown-linux-gnueabi"
+// CPU-A8: "-target-cpu" "cortex-a8"
+// EXT-CRC: "-target-feature" "+crc"

clang/test/Driver/arm-target-as-mthumb.s

@@ -5,12 +5,18 @@
 // RUN: %clang -target armv7a-linux-gnueabi -### -c -mthumb %s 2>&1 | \
 // RUN: FileCheck -check-prefix=TRIPLE-ARM %s
 // RUN: %clang -target armv7a-linux-gnueabi -### -c -Wa,-mthumb \
-// RUN: %S/Inputs/wildcard1.c  2>&1 | FileCheck -check-prefix=TRIPLE-ARM %s
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck -check-prefix=TRIPLE-ARM %s
+// RUN: %clang -target armv7a-linux-gnueabi -### -c -Wa,-mcpu=cortex-a8,-mthumb \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck -check-prefix=TRIPLE-ARM %s
 
 // TRIPLE-ARM: "-triple" "armv7-unknown-linux-gnueabi"
 
 // RUN: %clang -target armv7a-linux-gnueabi -### -c -Wa,-mthumb %s 2>&1 | \
 // RUN: FileCheck -check-prefix=TRIPLE-THUMB %s
+// RUN: %clang -target armv7a-linux-gnueabi -### -c -Wa,-mcpu=cortex-a8,-mthumb %s 2>&1 | \
+// RUN: FileCheck -check-prefix=TRIPLE-THUMB %s
+// RUN: %clang -target armv7a-linux-gnueabi -### -c -Wa,-mcpu=cortex-a8 -Wa,-mthumb %s 2>&1 | \
+// RUN: FileCheck -check-prefix=TRIPLE-THUMB %s
 // RUN: %clang -target armv7a-linux-gnueabi -### -c -Xassembler -mthumb %s \
 // RUN: 2>&1 | FileCheck -check-prefix=TRIPLE-THUMB %s