clang-cl: Simplify handling of /arch: flag.

[thakis]

Summary

r213083 initially implemented /arch: support by mapping it to CPU features. Then r241077 additionally mapped it to CPU, which made the feature flags redundant (if harmless). This change here removes the redundant mapping to feature flags, and rewrites test/Driver/cl-x86-flags.c to be a bit more of an integration test that checks for preprocessor defines like AVX (like documented on MSDN) instead of for driver flags.

To keep emitting warn_drv_unused_argument, use getLastArgNoClaim() followed by an explicit claim() if needed.

No intended behavior change.

This is in preparation for adding support for /arch:AVX512(F).

[hans]

Thanks for looking into this!

lib/Driver/ToolChains/Arch/X86.cpp

42–43

I wonder if it would work to do Args.getLastArgNoClaim() here instead, then do A->claim() if we actually use the argument, and let the general unused argument mechanism warn otherwise. Maybe that way we could avoid passing the Driver around.

144

Won't we need to set features for /arch:AVX512 though, or how will that work?

[thakis]

lib/Driver/ToolChains/Arch/X86.cpp

42–43

Hey that seems to work, nice.Passing the driver around is still kind of nice since it allows addressing the FIXME: above, but it shouldn't be part of this patch then. Done.

144

I'll just have to add

.Case("AVX512F", "knl")
.Case("AVX512", "skylake-avx512")

to the CPU switch above.

[hans]

lgtm

[thakis]

r323426, thanks!

lib/Driver/ToolChains/Arch/X86.cpp

if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
     // the host.
     //
     // FIXME: We should also incorporate the detected target features for use
     // with -native.
     std::string CPU = llvm::sys::getHostCPUName();
     if (!CPU.empty() && CPU != "generic")
       return Args.MakeArgString(CPU);
   }
 
-  if (const Arg *A = Args.getLastArg(options::OPT__SLASH_arch)) {
-    // Mapping built by referring to X86TargetInfo::getDefaultFeatures().
+  if (const Arg *A = Args.getLastArgNoClaim(options::OPT__SLASH_arch)) {
+    // Mapping built by looking at lib/Basic's X86TargetInfo::initFeatureMap().
     StringRef Arch = A->getValue();
     const char *CPU;
     if (Triple.getArch() == llvm::Triple::x86) {
       CPU = llvm::StringSwitch<const char *>(Arch)
                 .Case("IA32", "i386")
                 .Case("SSE", "pentium3")
                 .Case("SSE2", "pentium4")
                 .Case("AVX", "sandybridge")
                 .Case("AVX2", "haswell")
                 .Default(nullptr);
     } else {
       CPU = llvm::StringSwitch<const char *>(Arch)
                 .Case("AVX", "sandybridge")
                 .Case("AVX2", "haswell")
                 .Default(nullptr);
     }
-    if (CPU)
+    if (CPU) {
+      A->claim();
       return CPU;
     }
+  }
 
   // Select the default CPU if none was given (or detection failed).
 
   if (Triple.getArch() != llvm::Triple::x86_64 &&
       Triple.getArch() != llvm::Triple::x86)
     return nullptr; // This routine is only handling x86 targets.
 
   bool Is64Bit = Triple.getArch() == llvm::Triple::x86_64;
void x86::getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple,
   if (Triple.isAndroid()) {
     if (ArchType == llvm::Triple::x86_64) {
       Features.push_back("+sse4.2");
       Features.push_back("+popcnt");
     } else
       Features.push_back("+ssse3");
   }
 
-  // Set features according to the -arch flag on MSVC.
-  if (Arg *A = Args.getLastArg(options::OPT__SLASH_arch)) {
-    StringRef Arch = A->getValue();
-    bool ArchUsed = false;
-    // First, look for flags that are shared in x86 and x86-64.
-    if (ArchType == llvm::Triple::x86_64 || ArchType == llvm::Triple::x86) {
-      if (Arch == "AVX" || Arch == "AVX2") {
-        ArchUsed = true;
-        Features.push_back(Args.MakeArgString("+" + Arch.lower()));
-      }
-    }
-    // Then, look for x86-specific flags.
-    if (ArchType == llvm::Triple::x86) {
-      if (Arch == "IA32") {
-        ArchUsed = true;
-      } else if (Arch == "SSE" || Arch == "SSE2") {
-        ArchUsed = true;
-        Features.push_back(Args.MakeArgString("+" + Arch.lower()));
-      }
-    }
-    if (!ArchUsed)
-      D.Diag(clang::diag::warn_drv_unused_argument) << A->getAsString(Args);
-  }
-
   // Now add any that the user explicitly requested on the command line,
   // which may override the defaults.
   handleTargetFeaturesGroup(Args, Features, options::OPT_m_x86_Features_Group);
 }

test/Driver/cl-x86-flags.c

 // REQUIRES: x86-registered-target
 
+// expected-no-diagnostics
+
 // We support -m32 and -m64.  We support all x86 CPU feature flags in gcc's -m
 // flag space.
 // RUN: %clang_cl /Zs /WX -m32 -m64 -msse3 -msse4.1 -mavx -mno-avx \
 // RUN:     --target=i386-pc-win32 -### -- 2>&1 %s | FileCheck -check-prefix=MFLAGS %s
 // MFLAGS-NOT: argument unused during compilation
 
-// RUN: %clang_cl -m32 -arch:IA32 --target=i386 -### -- 2>&1 %s | FileCheck -check-prefix=IA32 %s
-// IA32: "-target-cpu" "i386"
-// IA32-NOT: -target-feature
-// IA32-NOT: argument unused during compilation
+// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-windows /c -Xclang -verify -DTEST_32_ARCH_IA32 -- %s
+#if defined(TEST_32_ARCH_IA32)
+#if _M_IX86_FP || __AVX__ || __AVX2__ || __AVX512F__  || __AVX512BW__
+#error fail
+#endif
+#endif
 
-// RUN: %clang_cl -m32 -arch:ia32 --target=i386 -### -- 2>&1 %s | FileCheck -check-prefix=ia32 %s
+// arch: args are case-sensitive.
+// RUN: %clang_cl -m32 -arch:ia32 --target=i386-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=ia32 %s
 // ia32: argument unused during compilation
-// ia32-NOT: -target-feature
 
-// RUN: %clang_cl -m64 -arch:IA32 --target=x86_64 -### -- 2>&1 %s | FileCheck -check-prefix=IA3264 %s
+// RUN: %clang_cl -m64 -arch:IA32 --target=x86_64-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=IA3264 %s
 // IA3264: argument unused during compilation
-// IA3264-NOT: -target-feature
 
-// RUN: %clang_cl -m32 -arch:SSE --target=i386 -### -- 2>&1 %s | FileCheck -check-prefix=SSE %s
-// SSE: "-target-cpu" "pentium3"
-// SSE: -target-feature
-// SSE: +sse
-// SSE-NOT: argument unused during compilation
+// RUN: %clang_cl -m32 -arch:SSE --target=i386-pc-windows /c -Xclang -verify -DTEST_32_ARCH_SSE -- %s
+#if defined(TEST_32_ARCH_SSE)
+#if _M_IX86_FP != 1 || __AVX__ || __AVX2__ || __AVX512F__  || __AVX512BW__
+#error fail
+#endif
+#endif
 
-// RUN: %clang_cl -m32 -arch:sse --target=i386 -### -- 2>&1 %s | FileCheck -check-prefix=sse %s
+// RUN: %clang_cl -m32 -arch:sse --target=i386-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=sse %s
 // sse: argument unused during compilation
-// sse-NOT: -target-feature
 
-// RUN: %clang_cl -m32 -arch:SSE2 --target=i386 -### -- 2>&1 %s | FileCheck -check-prefix=SSE2 %s
-// SSE2: "-target-cpu" "pentium4"
-// SSE2: -target-feature
-// SSE2: +sse2
-// SSE2-NOT: argument unused during compilation
+// RUN: %clang_cl -m32 -arch:SSE2 --target=i386-pc-windows /c -Xclang -verify -DTEST_32_ARCH_SSE2 -- %s
+#if defined(TEST_32_ARCH_SSE2)
+#if _M_IX86_FP != 2 || __AVX__ || __AVX2__ || __AVX512F__  || __AVX512BW__
+#error fail
+#endif
+#endif
 
-// RUN: %clang_cl -m32 -arch:sse2 --target=i386 -### -- 2>&1 %s | FileCheck -check-prefix=sse %s
+// RUN: %clang_cl -m32 -arch:sse2 --target=i386-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=sse %s
 // sse2: argument unused during compilation
-// sse2-NOT: -target-feature
 
-// RUN: %clang_cl -m64 -arch:SSE --target=x86_64 -### -- 2>&1 %s | FileCheck -check-prefix=SSE64 %s
+// RUN: %clang_cl -m64 -arch:SSE --target=x86_64-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=SSE64 %s
 // SSE64: argument unused during compilation
-// SSE64-NOT: -target-feature
-// SSE64-NOT: pentium3
 
-// RUN: %clang_cl -m64 -arch:SSE2 --target=x86_64 -### -- 2>&1 %s | FileCheck -check-prefix=SSE264 %s
+// RUN: %clang_cl -m64 -arch:SSE2 --target=x86_64-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=SSE264 %s
 // SSE264: argument unused during compilation
-// SSE264-NOT: -target-feature
 
-// RUN: %clang_cl -m32 -arch:AVX --target=i386 -### -- 2>&1 %s | FileCheck -check-prefix=AVX %s
-// AVX: "-target-cpu" "sandybridge"
-// AVX: -target-feature
-// AVX: +avx
+// RUN: %clang_cl -m32 -arch:AVX --target=i386-pc-windows /c -Xclang -verify -DTEST_32_ARCH_AVX -- %s
+#if defined(TEST_32_ARCH_AVX)
+#if _M_IX86_FP != 2 || !__AVX__ || __AVX2__ || __AVX512F__  || __AVX512BW__
+#error fail
+#endif
+#endif
 
-// RUN: %clang_cl -m32 -arch:avx --target=i386 -### -- 2>&1 %s | FileCheck -check-prefix=avx %s
+// RUN: %clang_cl -m32 -arch:avx --target=i386-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=avx %s
 // avx: argument unused during compilation
-// avx-NOT: -target-feature
 
-// RUN: %clang_cl -m32 -arch:AVX2 --target=i386 -### -- 2>&1 %s | FileCheck -check-prefix=AVX2 %s
-// AVX2: "-target-cpu" "haswell"
-// AVX2: -target-feature
-// AVX2: +avx2
+// RUN: %clang_cl -m32 -arch:AVX2 --target=i386-pc-windows /c -Xclang -verify -DTEST_32_ARCH_AVX2 -- %s
+#if defined(TEST_32_ARCH_AVX2)
+#if _M_IX86_FP != 2 || !__AVX__ || !__AVX2__ || __AVX512F__  || __AVX512BW__
+#error fail
+#endif
+#endif
 
-// RUN: %clang_cl -m32 -arch:avx2 --target=i386 -### -- 2>&1 %s | FileCheck -check-prefix=avx2 %s
+// RUN: %clang_cl -m32 -arch:avx2 --target=i386-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=avx2 %s
 // avx2: argument unused during compilation
-// avx2-NOT: -target-feature
 
-// RUN: %clang_cl -m64 -arch:AVX --target=x86_64 -### -- 2>&1 %s | FileCheck -check-prefix=AVX64 %s
-// AVX64: "-target-cpu" "sandybridge"
-// AVX64: -target-feature
-// AVX64: +avx
+// RUN: %clang_cl -m64 -arch:AVX --target=x86_64-pc-windows /c -Xclang -verify -DTEST_64_ARCH_AVX -- %s
+#if defined(TEST_64_ARCH_AVX)
+#if _M_IX86_FP || !__AVX__ || __AVX2__ || __AVX512F__  || __AVX512BW__
+#error fail
+#endif
+#endif
 
-// RUN: %clang_cl -m64 -arch:avx --target=x86_64 -### -- 2>&1 %s | FileCheck -check-prefix=avx64 %s
+// RUN: %clang_cl -m64 -arch:avx --target=x86_64-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=avx64 %s
 // avx64: argument unused during compilation
-// avx64-NOT: -target-feature
 
-// RUN: %clang_cl -m64 -arch:AVX2 --target=x86_64 -### -- 2>&1 %s | FileCheck -check-prefix=AVX264 %s
-// AVX264: "-target-cpu" "haswell"
-// AVX264: -target-feature
-// AVX264: +avx2
+// RUN: %clang_cl -m64 -arch:AVX2 --target=x86_64-pc-windows /c -Xclang -verify -DTEST_64_ARCH_AVX2 -- %s
+#if defined(TEST_64_ARCH_AVX2)
+#if _M_IX86_FP || !__AVX__ || !__AVX2__ || __AVX512F__  || __AVX512BW__
+#error fail
+#endif
+#endif
 
-// RUN: %clang_cl -m64 -arch:avx2 --target=x86_64 -### -- 2>&1 %s | FileCheck -check-prefix=avx264 %s
+// RUN: %clang_cl -m64 -arch:avx2 --target=x86_64-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=avx264 %s
 // avx264: argument unused during compilation
-// avx264-NOT: -target-feature
 
 void f() {
 }