Address review comments

Author: phoebewang

clang/include/clang/Driver/Options.td

@@ -9218,10 +9218,12 @@ def : CLFlag<"Qscatter-">, Alias<mno_scatter>,
 
 def _SLASH_arch : CLCompileJoined<"arch:">,
   HelpText<"Set architecture for code generation">;
-def _SLASH_vlen : CLCompileJoined<"vlen">,
-  HelpText<"Set vector length for autovectorization and other optimizations">;
-def _SLASH_vlen_default : CLFlag<"vlen">,
+def _SLASH_vlen : CLFlag<"vlen">,
   HelpText<"Set default vector length for autovectorization and other optimizations">;
+def _SLASH_vlen_EQ_256 : CLFlag<"vlen=256">,
+  HelpText<"Set vector length of 256 bits for autovectorization and other optimizations">;
+def _SLASH_vlen_EQ_512 : CLFlag<"vlen=512">,
+  HelpText<"Set vector length of 512 bits for autovectorization and other optimizations">;
 
 def _SLASH_M_Group : OptionGroup<"</M group>">, Group<cl_compile_Group>;
 def _SLASH_volatile_Group : OptionGroup<"</volatile group>">,

clang/lib/Driver/ToolChains/Clang.cpp

@@ -8266,34 +8266,28 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType,
                                                      << "/kernel";
  }
 
-  if (Args.hasArg(options::OPT__SLASH_vlen_default)) {
-    // Override /vlen= options.
-    for (const Arg *A : Args.filtered(options::OPT__SLASH_vlen))
-      A->claim();
-    // Nothing to do for AVX512F/AVX512.
-  } else if (const Arg *A = Args.getLastArg(options::OPT__SLASH_vlen)) {
-    StringRef VLen = A->getValue();
+  if (const Arg *A = Args.getLastArg(options::OPT__SLASH_vlen,
+                                     options::OPT__SLASH_vlen_EQ_256,
+                                     options::OPT__SLASH_vlen_EQ_512)) {
     llvm::SmallSet<std::string, 4> Arch512 = {"AVX512F", "AVX512"};
     llvm::SmallSet<std::string, 4> Arch256 = {"AVX", "AVX2"};
 
     StringRef Arch = Args.getLastArgValue(options::OPT__SLASH_arch);
-    if (Arch != "") {
-      if (VLen == "=512") {
-        if (Arch512.contains(Arch.str()))
-          CmdArgs.push_back("-mprefer-vector-width=512");
-        else
-          D.Diag(diag::warn_drv_argument_not_allowed_with)
-              << "/vlen=512" << std::string("/arch:").append(Arch);
-      } else if (VLen == "=256") {
-        if (Arch512.contains(Arch.str()))
-          CmdArgs.push_back("-mprefer-vector-width=256");
-        else if (!Arch256.contains(Arch.str()))
-          D.Diag(diag::warn_drv_argument_not_allowed_with)
-              << "/vlen=256" << std::string("/arch:").append(Arch);
-      } else {
-          D.Diag(diag::warn_drv_unknown_argument_clang_cl)
-              << std::string("/vlen").append(VLen);
-      }
+
+    if (A->getOption().matches(options::OPT__SLASH_vlen_EQ_512)) {
+      if (Arch512.contains(Arch.str()))
+        CmdArgs.push_back("-mprefer-vector-width=512");
+      else
+        D.Diag(diag::warn_drv_argument_not_allowed_with)
+            << "/vlen=512" << std::string("/arch:").append(Arch);
+    }
+
+    if (A->getOption().matches(options::OPT__SLASH_vlen_EQ_256)) {
+      if (Arch512.contains(Arch.str()))
+        CmdArgs.push_back("-mprefer-vector-width=256");
+      else if (!Arch256.contains(Arch.str()))
+        D.Diag(diag::warn_drv_argument_not_allowed_with)
+            << "/vlen=256" << std::string("/arch:").append(Arch);
     }
   }
 

clang/test/Driver/cl-x86-flags.c

@@ -151,9 +151,6 @@
 // RUN: %clang_cl -m32 -arch:SSE2 -vlen=256 --target=i386-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=sse2vlen256 %s
 // sse2vlen256: invalid argument '/vlen=256' not allowed with '/arch:SSE2'
 
-// RUN: %clang_cl -m64 -arch:AVX -vlen256 --target=x86_64-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=avxvlen256 %s
-// avxvlen256: unknown argument ignored in clang-cl: '/vlen256'
-
 void f(void) {
 }