[PATCH] D42497: clang-cl: Simplify handling of /arch: flag.

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.


================
Comment at: lib/Driver/ToolChains/Arch/X86.cpp:43
 
   if (const Arg *A = Args.getLastArg(options::OPT__SLASH_arch)) {
+    // Mapping built by looking at lib/Basic's X86TargetInfo::initFeatureMap().
----------------
hans wrote:
> 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.
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.


================
Comment at: lib/Driver/ToolChains/Arch/X86.cpp:144
 
-  // Set features according to the -arch flag on MSVC.
-  if (Arg *A = Args.getLastArg(options::OPT__SLASH_arch)) {
----------------
hans wrote:
> Won't we need to set features for /arch:AVX512 though, or how will that work?
I'll just have to add

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

to the CPU switch above.


https://reviews.llvm.org/D42497