commit 1d51c699b9e2ebc5bcfdbe85c74cc871426333d4
Author: David Spickett <david.spickett@linaro.org>
Date:   Mon Feb 1 11:29:14 2021 +0000

    [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

diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index b2ddef141a75..c83638086048 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/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
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index ef590db1eecd..d0606eb882f1 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/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);
 
diff --git a/clang/test/Driver/arm-target-as-march-mcpu.s b/clang/test/Driver/arm-target-as-march-mcpu.s
new file mode 100644
index 000000000000..31c027bd0fa6
--- /dev/null
+++ b/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"
diff --git a/clang/test/Driver/arm-target-as-mthumb.s b/clang/test/Driver/arm-target-as-mthumb.s
index 7e014bb9d43f..4bfef8dbf235 100644
--- a/clang/test/Driver/arm-target-as-mthumb.s
+++ b/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