Reland "[AArch64] handle -Wa,-march="

[jcai19]

Summary

This reverts commit fd11a26d368c5a909fb88548fef2cee7a6c2c931, which was
reverted by 9145a3d4ab7eb05d9fb113b5392e8961df629b88 due to a test
failure on aarch64 backend, e.g.
https://lab.llvm.org/buildbot/#/builders/43/builds/7031. This patch
fixed the test failure.

[joerg]

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

230

This comment is confusing. -march is a driver option and the GCC driver will internally convert it into the equivalent of -Wa,-march before passing the total command line to GNU as.

[jcai19]

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

230

Thanks for the clarification! I haven't checked GCC implementation so the comment is likely not accurate. I did make the following observation from below command and its output. It seems all the -Wa.-march values passed (armv8.3-a and armv8.4a) showed up in COLLECT_AS_OPTIONS, as well as in COLLECT_GCC_OPTIONS. On the other hand, only the last value of -march (armv8.2-a) showed up in COLLECT_GCC_OPTIONS, and the -march value was placed before all the -Wa,-march values in COLLECT_GCC_OPTIONS. WDYT?

$ aarch64-linux-gnu-gcc -Wa,-march=armv8.1-a -march=armv8.2-a -march=armv8.3-a -Wa,-march=armv8.4-a -c foo.s -o gcc.o -###
Using built-in specs.
COLLECT_AS_OPTIONS='-march=armv8.1-a' '-march=armv8.4-a'
COLLECT_GCC=aarch64-linux-gnu-gcc
Target: aarch64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 10.2.1-6+build2' --with-bugurl=file:///usr/share/doc/gcc-10/README.Bugs --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-10 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-libquadmath --disable-libquadmath-support --enable-plugin --enable-default-pie --with-system-zlib --enable-libphobos-checking=release --without-target-system-zlib --enable-multiarch --enable-fix-cortex-a53-843419 --disable-werror --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=aarch64-linux-gnu --program-prefix=aarch64-linux-gnu- --includedir=/usr/aarch64-linux-gnu/include --with-build-config=bootstrap-lto-lean --enable-link-mutex
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 10.2.1 20210110 (Debian 10.2.1-6+build2)
COLLECT_GCC_OPTIONS='-march=armv8.3-a' '-c' '-o' 'gcc.o' '-mlittle-endian' '-mabi=lp64'
/usr/lib/gcc-cross/aarch64-linux-gnu/10/../../../../aarch64-linux-gnu/bin/as -EL "-march=armv8.3-a" "-mabi=lp64" "-march=armv8.1-a" "-march=armv8.4-a" -o gcc.o foo.s
COMPILER_PATH=/usr/lib/gcc-cross/aarch64-linux-gnu/10/:/usr/lib/gcc-cross/aarch64-linux-gnu/10/:/usr/lib/gcc-cross/aarch64-linux-gnu/:/usr/lib/gcc-cross/aarch64-linux-gnu/10/:/usr/lib/gcc-cross/aarch64-linux-gnu/:/usr/lib/gcc-cross/aarch64-linux-gnu/10/../../../../aarch64-linux-gnu/bin/
LIBRARY_PATH=/usr/lib/gcc-cross/aarch64-linux-gnu/10/:/usr/lib/gcc-cross/aarch64-linux-gnu/10/../../../../aarch64-linux-gnu/lib/../lib/:/lib/aarch64-linux-gnu/:/lib/../lib/:/usr/lib/aarch64-linux-gnu/:/usr/lib/../lib/:/usr/lib/gcc-cross/aarch64-linux-gnu/10/../../../../aarch64-linux-gnu/lib/:/lib/:/usr/lib/
COLLECT_GCC_OPTIONS='-march=armv8.3-a' '-c' '-o' 'gcc.o' '-mlittle-endian' '-mabi=lp64'

[DavidSpickett]

Thanks for taking this up! I never got the time for it.

I'd like to see the test check that there are no unused argument warnings when there are multiple values. I missed that with another patch in this area.

Beyond that I'm not sure the logic is right here. You've got -Wa,-march being additive, in contrast to the compiler option which only uses the last value:

$ ./bin/clang --target=aarch64-arm-none-eabi -march=armv8.1-a -march=armv8.2-a /tmp/test.c -###
<...> "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.2a" <...>

Maybe you're working to make some specific build system work but from the clang side we'd want consistent behaviour for Arm and AArch64 options.

Let me restate the Arm behaviour, minus the mcpu stuff that was also in that patch:

• Only compiler options apply to non assembly files
• Compiler and assembler options apply to assembly files
• For assembly files, if you have both kinds of option, you prefer the assembler option(s)
• Of the options that apply (or are preferred), the last value wins (it's not additive)

Do you disagree with that set of rules?

I wouldn't think that AArch64 would have that different needs, but then again I'm not trying to build existing projects, just make the clang side logical.

[jcai19]

Thanks for the comments. Please see my replies inlined.

In D103184#2790486, @DavidSpickett wrote:

Thanks for taking this up! I never got the time for it.

I'd like to see the test check that there are no unused argument warnings when there are multiple values. I missed that with another patch in this area.

Beyond that I'm not sure the logic is right here. You've got -Wa,-march being additive, in contrast to the compiler option which only uses the last value:

$ ./bin/clang --target=aarch64-arm-none-eabi -march=armv8.1-a -march=armv8.2-a /tmp/test.c -###
<...> "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.2a" <...>

Maybe you're working to make some specific build system work but from the clang side we'd want consistent behaviour for Arm and AArch64 options.

I understand it's a little bit confusing here, but I was simply trying to match GCC's behavior (please see the example in my last comment) unless I misunderstood its output. I definitely agree having consistent behaviors between Arm and Aarch64 in Clang is more reasonable (in fact that was what I implemented at first) and maybe we should fork from gcc, WDYT?

Let me restate the Arm behaviour, minus the mcpu stuff that was also in that patch:

• Only compiler options apply to non assembly files
• Compiler and assembler options apply to assembly files
• For assembly files, if you have both kinds of option, you prefer the assembler option(s)
• Of the options that apply (or are preferred), the last value wins (it's not additive)

Do you disagree with that set of rules?

I wouldn't think that AArch64 would have that different needs, but then again I'm not trying to build existing projects, just make the clang side logical.

Thanks for the clarification. I will add more checks.

[nickdesaulniers]

clang/test/Driver/aarch64-target-as-march.s

5

https://reviews.llvm.org/D95872 tests were pretty extensive. We don't need -mcpu tests like https://reviews.llvm.org/D95872 had, but I think it might be interesting to add more tests, particularly one that verifies -Wa,-march is overriding -march.

[jcai19]

Limit the change to assembler files, and add more tests.

[jcai19]

clang/test/Driver/aarch64-target-as-march.s

5

Sounds good! I've added more tests which covered the above-mentioned case.

[DavidSpickett]

I understand it's a little bit confusing here, but I was simply trying to match GCC's behavior (please see the example in my last comment) unless I misunderstood its output. I definitely agree having consistent behaviors between Arm and Aarch64 in Clang is more reasonable (in fact that was what I implemented at first) and maybe we should fork from gcc, WDYT?

If gas handles march by choosing the last value, then we don't need to emit multiple values just to look more like GCC's processed arguments.

Let me check what gas does when you run it directly, I think that's the best way to guide things.

[DavidSpickett]

$ cat /tmp/test.s
irg x0, x0
$ aarch64-unknown-linux-gnu-as -march=armv8.5-a+memtag -march=armv8.1-a /tmp/test.s -o /dev/null
/tmp/test.s: Assembler messages:
/tmp/test.s:1: Error: selected processor does not support `irg x0,x0'

GAS is also taking the last value of march. So if we were to follow clang Arm's behaviour, the result would be the same.

Except we don't have to rely on the llvm backend picking the last of "target-feature". Which probably works for architecture versions but for individual extensions I don't think so.

If we make it additive this could happen:

-march=armv8.5-a+memtag -march=armv8.2-a

Becomes: (mte is the backend name for memtag for silly reasons)

-target-feature +v8.5-a -target-feature mte -target-feature +v8.2-a

So now instead of getting v8.2 only, the user now has v8.2+memtag. Only if they do armv8.2-a+nomemtag will they get what they expect (well, what I'd expect). Which means they'd need to know every previous march value.

[jcai19]

In D103184#2793055, @DavidSpickett wrote:

$ cat /tmp/test.s
irg x0, x0
$ aarch64-unknown-linux-gnu-as -march=armv8.5-a+memtag -march=armv8.1-a /tmp/test.s -o /dev/null
/tmp/test.s: Assembler messages:
/tmp/test.s:1: Error: selected processor does not support `irg x0,x0'

GAS is also taking the last value of march. So if we were to follow clang Arm's behaviour, the result would be the same.

Except we don't have to rely on the llvm backend picking the last of "target-feature". Which probably works for architecture versions but for individual extensions I don't think so.

If we make it additive this could happen:

-march=armv8.5-a+memtag -march=armv8.2-a

Becomes: (mte is the backend name for memtag for silly reasons)

-target-feature +v8.5-a -target-feature mte -target-feature +v8.2-a

So now instead of getting v8.2 only, the user now has v8.2+memtag. Only if they do armv8.2-a+nomemtag will they get what they expect (well, what I'd expect). Which means they'd need to know every previous march value.

Thanks for double checking! I've changed my implementation since I made that comment to take the last value of -Wa,march (and only for assembly files) and added more test cases. It should now follows the Arm behavior.

[nickdesaulniers]

clang/test/Driver/aarch64-target-as-march.s

21–26

perhaps add a test with 8.1 and 8.2 in opposite order that 8.1 is chosen, not 8.2?

[jcai19]

Add a test case.

[nickdesaulniers]

LGTM; any additional thoughts @DavidSpickett ?

[DavidSpickett]

LGTM; any additional thoughts @DavidSpickett ?

A couple of additional tests just to be safe but otherwise LGTM.

Also please include in the commit message the "rules" that we're using here for parsing. Like I did for the arm change.

clang/test/Driver/aarch64-target-as-march.s

30

Add a test with -Wa,-march=armv8.2-a,-march=armv8.1-a (one -Wa with multiple values attached), then repeat the two tests but with -Xassembler instead.

[jcai19]

Address the comments and add more tests.

[jcai19]

In D103184#2798398, @DavidSpickett wrote:

LGTM; any additional thoughts @DavidSpickett ?

A couple of additional tests just to be safe but otherwise LGTM.

Also please include in the commit message the "rules" that we're using here for parsing. Like I did for the arm change.

Thanks all!

clang/test/Driver/aarch64-target-as-march.s

30

I added a test case for -Xassembler -march=armv8.2-a,-march=armv8.1-a, but it seems it does not support multiple values in one invocation?

clang --target=aarch64-linux-gnueabi -Xassembler -march=armv8.2-a,-march=armv8.1-a -c foo.s -o ias.o -###
...
clang-13: error: the clang compiler does not support '-Xassembler -march=armv8.2-a,-march=armv8.1-a'
...

[nickdesaulniers]

clang/test/Driver/aarch64-target-as-march.s

30

I don't think @DavidSpickett meant more UNUSED-WARNING tests, but tests that check the actual chosen value.

[jcai19]

Update test cases based on comments.

[nickdesaulniers]

That looks better, thanks @jcai19 . Based on @DavidSpickett 's LGTM I think this is now ready to land.

@DavidSpickett did ask:

Also please include in the commit message the "rules" that we're using here for parsing. Like I did for the arm change.

So you may want to edit the commit description manually in phab if you plan to arc patch this in before merging, otherwise please do so locally before merging to main then pushing.

clang/test/Driver/aarch64-target-as-march.s

30

I added a test case for -Xassembler -march=armv8.2-a,-march=armv8.1-a, but it seems it does not support multiple values in one invocation?

Looking at https://reviews.llvm.org/D95872, your assessment seems correct.

[jcai19]

In D103184#2799732, @nickdesaulniers wrote:

That looks better, thanks @jcai19 . Based on @DavidSpickett 's LGTM I think this is now ready to land.

@DavidSpickett did ask:

Also please include in the commit message the "rules" that we're using here for parsing. Like I did for the arm change.

So you may want to edit the commit description manually in phab if you plan to arc patch this in before merging, otherwise please do so locally before merging to main then pushing.

Thanks for the confirmation! I've edited the commit message to include the rules per discussed. Will wait till Monday in case @DavidSpickett has more comments.

clang/test/Driver/aarch64-target-as-march.s

30

That makes more sense. Sorry for the noise.

[DavidSpickett]

LGTM

[thakis]

This breaks tests on arm macs: http://45.33.8.238/macm1/10931/step_7.txt

Please take a look and revert for now if it takes a while to fix.

[jcai19]

In D103184#2803513, @thakis wrote:

This breaks tests on arm macs: http://45.33.8.238/macm1/10931/step_7.txt

Please take a look and revert for now if it takes a while to fix.

Thanks, will address the breakage ASAP.

[nickdesaulniers]

In D103184#2803513, @thakis wrote:

This breaks tests on arm macs: http://45.33.8.238/macm1/10931/step_7.txt

clang: error: the clang compiler does not support '-Wa,--version'

Wouldn't that be https://reviews.llvm.org/rG76d9bc72784d88f4dd57b9939e52c73739438af5 (https://reviews.llvm.org/D99556)?

[thakis]

In D103184#2803691, @nickdesaulniers wrote:

In D103184#2803513, @thakis wrote:

This breaks tests on arm macs: http://45.33.8.238/macm1/10931/step_7.txt

clang: error: the clang compiler does not support '-Wa,--version'

Wouldn't that be https://reviews.llvm.org/rG76d9bc72784d88f4dd57b9939e52c73739438af5 (https://reviews.llvm.org/D99556)?

First bad build is http://45.33.8.238/macm1/10923/summary.html which has https://github.com/llvm/llvm-project/compare/1465e7770bca...43929ccc1296 as regression range

[thakis]

FWIW the failure goes away locally if I revert this change here, so it's definitely due to this change here.

Things have been red for a while, probably good to revert while you investigate by this point.

[jcai19]

In D103184#2803768, @thakis wrote:

FWIW the failure goes away locally if I revert this change here, so it's definitely due to this change here.

Things have been red for a while, probably good to revert while you investigate by this point.

Sorry for the delay but my macbook had some issues so I just finished building LLVM, but I can't seem to reproduce the test failure locally (neither by running the test directly with llvm-lit nor with ninja check-clang). In fact I don't quite understand why this test failed. Like @nickdesaulniers mentioned the build failure should have been fixed in https://reviews.llvm.org/rG76d9bc72784d88f4dd57b9939e52c73739438af5 for a different patch. Do you mind sharing the instructions you used to reproduce the test failure? Thanks.

[thakis]

In D103184#2804241, @jcai19 wrote:

In D103184#2803768, @thakis wrote:

FWIW the failure goes away locally if I revert this change here, so it's definitely due to this change here.

Things have been red for a while, probably good to revert while you investigate by this point.

Sorry for the delay but my macbook had some issues so I just finished building LLVM, but I can't seem to reproduce the test failure locally (neither by running the test directly with llvm-lit nor with ninja check-clang). In fact I don't quite understand why this test failed. Like @nickdesaulniers mentioned the build failure should have been fixed in https://reviews.llvm.org/rG76d9bc72784d88f4dd57b9939e52c73739438af5 for a different patch. Do you mind sharing the instructions you used to reproduce the test failure? Thanks.

Did you try on a mac with a m1 processor?

[nickdesaulniers]

In D103184#2813748, @thakis wrote:

In D103184#2804241, @jcai19 wrote:

In D103184#2803768, @thakis wrote:

FWIW the failure goes away locally if I revert this change here, so it's definitely due to this change here.

Things have been red for a while, probably good to revert while you investigate by this point.

Sorry for the delay but my macbook had some issues so I just finished building LLVM, but I can't seem to reproduce the test failure locally (neither by running the test directly with llvm-lit nor with ninja check-clang). In fact I don't quite understand why this test failed. Like @nickdesaulniers mentioned the build failure should have been fixed in https://reviews.llvm.org/rG76d9bc72784d88f4dd57b9939e52c73739438af5 for a different patch. Do you mind sharing the instructions you used to reproduce the test failure? Thanks.

Did you try on a mac with a m1 processor?

This suggestion doesn't make sense to me. The order of events:

1. https://reviews.llvm.org/rG3cc3c0f8352ec33ca2f2636f94cb1d85fc57ac16 landed, broke the build
2. 1 was reverted in https://reviews.llvm.org/rGbf2479c347c8ca88fefdb144d8bae0a7a4231e2a
3. 1 was relanded in https://reviews.llvm.org/rG76d9bc72784d88f4dd57b9939e52c73739438af5
4. different patch was landed https://reviews.llvm.org/rGfd11a26d368c5a909fb88548fef2cee7a6c2c931
5. @thakis reports patch from 4 causes a test failure, but the test failure looks like the test failure from 1, not 4.
6. 4 is reverted in https://reviews.llvm.org/rG9145a3d4ab7eb05d9fb113b5392e8961df629b88

#5 doesn't make sense to me, and I don't see how the test would turn out differently on an m1 mac vs x86

[jcai19]

The original implementation was indeed flawed. The failed test does not specify target triple so on X86 machines the issue won't happen unless I explicitly specify "-target aarch64".
This fixes the problem.

[jcai19]

It's probably clearer to use a new differential review for relanding. I've created https://reviews.llvm.org/D104656. Sorry for the noises.

[jcai19]

Use the original flawed code for future reference.

[nickdesaulniers]

removing LGTM until the reland is posted for review here so we can use the history tab to observe the changes.

[jcai19]

After chatting with @nickdesaulniers offline, we have decided to abondoned D104656 in favor of keeping all the updates to this differential review for consistency. Appologies for any inconvenience caused.

[nickdesaulniers]

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

201–205

If this surprised us before, it's likely to surprise us (or someone else) again. In that case, I'd recommend permitting getAArch64ArchFeaturesFromMarch to accept an empty second parameter, and simply return true early. Then callers don't need such a check and comment.

[nickdesaulniers]

Thanks for following up on a fix!

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

201–205

@jcai19 points out this is how it's done for arm32 as well:
https://reviews.llvm.org/D95872

[jcai19]

In D103184#2831542, @nickdesaulniers wrote:

Thanks for following up on a fix!

Thanks!

clang/lib/Driver/ToolChains/Arch/AArch64.h

 
 namespace clang {
 namespace driver {
 namespace tools {
 namespace aarch64 {
 
 void getAArch64TargetFeatures(const Driver &D, const llvm::Triple &Triple,
                               const llvm::opt::ArgList &Args,
-                              std::vector<llvm::StringRef> &Features);
+                              std::vector<llvm::StringRef> &Features,
+                              bool ForAS);
 
 std::string getAArch64TargetCPU(const llvm::opt::ArgList &Args,
                                 const llvm::Triple &Triple, llvm::opt::Arg *&A);
 
 } // end namespace aarch64
 } // end namespace target
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_ARCH_AARCH64_H

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

if (!DecodeAArch64Mcpu(D, McpuLowerCase, CPU, DecodedFeature))
     return false;
 
   return getAArch64MicroArchFeaturesFromMtune(D, CPU, Args, Features);
 }
 
 void aarch64::getAArch64TargetFeatures(const Driver &D,
                                        const llvm::Triple &Triple,
                                        const ArgList &Args,
-                                       std::vector<StringRef> &Features) {
+                                       std::vector<StringRef> &Features,
+                                       bool ForAS) {
   Arg *A;
   bool success = true;
   // Enable NEON by default.
   Features.push_back("+neon");
-  if ((A = Args.getLastArg(options::OPT_march_EQ)))
+  llvm::StringRef WaMArch = "";
+  if (ForAS)
+    for (const auto *A :
+         Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler))
+      for (StringRef Value : A->getValues())
+        if (Value.startswith("-march="))
+          WaMArch = Value.substr(7);
+  // Call getAArch64ArchFeaturesFromMarch only if "-Wa,-march=" or
+  // "-Xassembler -march" is detected. Otherwise it may return false
+  // and causes Clang to error out.
+  if (WaMArch.size())
+    success = getAArch64ArchFeaturesFromMarch(D, WaMArch, Args, Features);
+  else if ((A = Args.getLastArg(options::OPT_march_EQ)))
     success = getAArch64ArchFeaturesFromMarch(D, A->getValue(), Args, Features);
   else if ((A = Args.getLastArg(options::OPT_mcpu_EQ)))
     success = getAArch64ArchFeaturesFromMcpu(D, A->getValue(), Args, Features);
   else if (Args.hasArg(options::OPT_arch) || isCPUDeterminedByTriple(Triple))
     success = getAArch64ArchFeaturesFromMcpu(
         D, getAArch64TargetCPU(Args, Triple, A), Args, Features);
 
   if (success && (A = Args.getLastArg(clang::driver::options::OPT_mtune_EQ)))
     success =
         getAArch64MicroArchFeaturesFromMtune(D, A->getValue(), Args, Features);
   else if (success && (A = Args.getLastArg(options::OPT_mcpu_EQ)))
     success =
         getAArch64MicroArchFeaturesFromMcpu(D, A->getValue(), Args, Features);
   else if (success &&
            (Args.hasArg(options::OPT_arch) || isCPUDeterminedByTriple(Triple)))
     success = getAArch64MicroArchFeaturesFromMcpu(
         D, getAArch64TargetCPU(Args, Triple, A), Args, Features);
 
   if (!success)
     D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
 
   if (Args.getLastArg(options::OPT_mgeneral_regs_only)) {
     Features.push_back("-fp-armv8");
     Features.push_back("-crypto");
     Features.push_back("-neon");
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_mtp_mode_EQ)) {
     StringRef Mtp = A->getValue();
     if (Mtp == "el3")
       Features.push_back("+tpidr-el3");
     else if (Mtp == "el2")

clang/lib/Driver/ToolChains/Clang.cpp

case llvm::Triple::riscv64:
     riscv::getRISCVTargetFeatures(D, Triple, Args, Features);
     break;
   case llvm::Triple::systemz:
     systemz::getSystemZTargetFeatures(D, Args, Features);
     break;
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_32:
   case llvm::Triple::aarch64_be:
-    aarch64::getAArch64TargetFeatures(D, Triple, Args, Features);
+    aarch64::getAArch64TargetFeatures(D, Triple, Args, Features, ForAS);
     break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
     x86::getX86TargetFeatures(D, Triple, Args, Features);
     break;
   case llvm::Triple::hexagon:
     hexagon::getHexagonTargetFeatures(D, Args, Features);
     break;

clang/test/Driver/aarch64-target-as-march.s

+/// These tests make sure that options passed to the assembler
+/// via -Wa or -Xassembler are applied correctly to assembler inputs.
+
+/// Does not apply to non assembly files
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.1-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-1 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.1-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-1 %s
+
+// TARGET-FEATURE-1-NOT: "-target-feature" "+v8.1a"
+
+/// Does apply to assembler input
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TARGET-FEATURE-2 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TARGET-FEATURE-2 %s
+
+// TARGET-FEATURE-2: "-target-feature" "+v8.2a"
+
+/// No unused argument warnings when there are multiple values
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.1-a -Wa,-march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=UNUSED-WARNING %s
+
+// UNUSED-WARNING-NOT: warning: argument unused during compilation
+
+/// Last march to assembler wins
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.2-a -Wa,-march=armv8.1-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.2-a,-march=armv8.1-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.2-a -Xassembler \
+// RUN: -march=armv8.1-a %s 2>&1 | FileCheck --check-prefix=MULTIPLE-VALUES %s
+
+// MULTIPLE-VALUES: "-target-feature" "+v8.1a
+// MULTIPLE-VALUES-NOT: "-target-feature" "+v8.2a
+
+/// march to compiler and assembler, we choose the one suited to the input file type
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TARGET-FEATURE-3 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-4 %s
+
+// TARGET-FEATURE-3: "-target-feature" "+v8.3a"
+// TARGET-FEATURE-3-NOT: "-target-feature" "+v8.4a"
+// TARGET-FEATURE-4: "-target-feature" "+v8.4a"
+// TARGET-FEATURE-4-NOT: "-target-feature" "+v8.3a"