[ARM][Driver][Windows] Allow command-line upgrade to Armv8.

[simon_tatham]

Summary

If you gave clang the options --target=arm-pc-windows-msvc and
-march=armv8-a+crypto together, the crypto extension would not be
enabled in the compilation, and you'd see the following warning
message suggesting that the 'armv8-a' had been ignored:

clang: warning: ignoring extension 'crypto' because the 'armv7-a' architecture does not support it [-Winvalid-command-line-argument]

This happens because Triple::getARMCPUForArch(), for the Win32 OS,
unconditionally returns "cortex-a9" (an Armv7 CPU) regardless of
MArch, which overrides the architecture setting on the command line.

I don't think that the combination of Windows and AArch32 _should_
unconditionally outlaw the use of the crypto extension. MSVC itself
doesn't think so: you can perfectly well compile Thumb crypto code
using its AArch32-targeted compiler.

All the other default CPUs in the same switch statement are
conditional on a particular MArch setting; this is the only one that
returns a particular CPU _regardless_ of MArch. So I've fixed this one
by adding a condition, so that if you ask for an architecture *above*
v7, the default of Cortex-A9 no longer overrides it.

[mstorsjo]

LGTM, thanks!

clang/test/Driver/woa-crypto.c

3

Nitpick: Is it possible to test this part somewhere using the tools in the llvm subset of the repo? That'd keep the changes a bit more localized?

[simon_tatham]

Yes, it looks easy enough to add something in llvm/unittests/ADT/TripleTest.cpp to directly test getARMCPUForArch.

I'd mildly prefer to do that as well as having the test here, because the call site in the clang driver is quite complicated. My real aim is that asking for crypto (or any other Armv8-specific extension) should actually get it; I think any accidental regression of that behavior in future is at least as likely to arise from changes in clang/lib/Driver as llvm/lib/Support, so I'd like there to be a test that will detect the regression if it occurs for any reason.

[mstorsjo]

In D100937#2704530, @simon_tatham wrote:

Yes, it looks easy enough to add something in llvm/unittests/ADT/TripleTest.cpp to directly test getARMCPUForArch.

I'd mildly prefer to do that as well as having the test here, because the call site in the clang driver is quite complicated. My real aim is that asking for crypto (or any other Armv8-specific extension) should actually get it; I think any accidental regression of that behavior in future is at least as likely to arise from changes in clang/lib/Driver as llvm/lib/Support, so I'd like there to be a test that will detect the regression if it occurs for any reason.

Fair enough, that sounds like a reasonable plan to me. Yeah having tests for the actual end results is kinda nice instead of just small broken up unit tests, especially when it isn't something that shouldn't change spuriously (like generated code).

[simon_tatham]

Added a unit test on the LLVM side.

[mstorsjo]

Thank you!

clang/test/Driver/woa-crypto.c

+// RUN: %clang -target arm-windows-msvc                       -### -S %s -O0 -o /dev/null 2>&1 | FileCheck %s -check-prefix CHECK-DEFAULT
+// RUN: %clang -target arm-windows-msvc -march=armv8-a+crypto -### -S %s -O0 -o /dev/null 2>&1 | FileCheck %s -check-prefix CHECK-CRYPTO
+
+// CHECK-DEFAULT: "-target-cpu" "cortex-a9"
+// CHECK-CRYPTO: "-target-cpu" "generic"
+// CHECK-CRYPTO: "-target-feature" "+crypto"

llvm/lib/Support/Triple.cpp

StringRef Triple::getARMCPUForArch(StringRef MArch) const {
   case llvm::Triple::NetBSD:
     if (!MArch.empty() && MArch == "v6")
       return "arm1176jzf-s";
     if (!MArch.empty() && MArch == "v7")
       return "cortex-a8";
     break;
   case llvm::Triple::Win32:
     // FIXME: this is invalid for WindowsCE
+    if (ARM::parseArchVersion(MArch) <= 7)
       return "cortex-a9";
+    break;
   case llvm::Triple::IOS:
   case llvm::Triple::MacOSX:
   case llvm::Triple::TvOS:
   case llvm::Triple::WatchOS:
     if (MArch == "v7k")
       return "cortex-a7";
     break;
   default:

llvm/unittests/ADT/TripleTest.cpp

TEST(TripleTest, getARMCPUForArch) {
   }
   {
     llvm::Triple Triple("armebv6-unknown-freebsd");
     EXPECT_EQ("arm1176jzf-s", Triple.getARMCPUForArch());
   }
   {
     llvm::Triple Triple("arm--win32");
     EXPECT_EQ("cortex-a9", Triple.getARMCPUForArch());
+    EXPECT_EQ("generic", Triple.getARMCPUForArch("armv8-a"));
   }
   // Some alternative architectures
   {
     llvm::Triple Triple("armv7k-apple-ios9");
     EXPECT_EQ("cortex-a7", Triple.getARMCPUForArch());
   }
   {
     llvm::Triple Triple("armv7k-apple-watchos3");