[clang-cl] Handle -O correctly

We had multiple bugs here:
- We didn't support multiple optimization options in one argument.
  e.g. -O2y-
- We didn't correctly expand -O[12dx] to their respective options.
- We treated -O1 as clang -O1 instead of clang -Os.
- We treated -Ox as clang -O3 instead of clang -O2.  In fact, cl's -Ox
  option is *less* powerful than cl's -O2 option despite -Ox described
  as "Full Optimization".

This fixes PR24003.

llvm-svn: 243261

Author: majnemer

clang/include/clang/Driver/CLCompatOptions.td

@@ -92,8 +92,7 @@ def _SLASH_I : CLJoinedOrSeparate<"I">,
 def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,
   Alias<funsigned_char>;
 def _SLASH_O0 : CLFlag<"O0">, Alias<O0>;
-def _SLASH_O : CLJoined<"O">, HelpText<"Optimization level">,
-  MetaVarName<"<n>">, Alias<O>;
+def _SLASH_O : CLJoined<"O">, HelpText<"Optimization level">;
 def _SLASH_Ob0 : CLFlag<"Ob0">, HelpText<"Disable inlining">,
   Alias<fno_inline>;
 def _SLASH_Od : CLFlag<"Od">, HelpText<"Disable optimization">, Alias<O0>;
@@ -105,8 +104,6 @@ def _SLASH_Os : CLFlag<"Os">, HelpText<"Optimize for size">, Alias<O>,
   AliasArgs<["s"]>;
 def _SLASH_Ot : CLFlag<"Ot">, HelpText<"Optimize for speed">, Alias<O>,
   AliasArgs<["2"]>;
-def _SLASH_Ox : CLFlag<"Ox">, HelpText<"Maximum optimization">, Alias<O>,
-  AliasArgs<["3"]>;
 def _SLASH_Oy : CLFlag<"Oy">, HelpText<"Enable frame pointer omission">,
   Alias<fomit_frame_pointer>;
 def _SLASH_Oy_ : CLFlag<"Oy-">, HelpText<"Disable frame pointer omission">,
@@ -261,6 +258,7 @@ def _SLASH_kernel_ : CLIgnoredFlag<"kernel-">;
 def _SLASH_nologo : CLIgnoredFlag<"nologo">;
 def _SLASH_Ob1 : CLIgnoredFlag<"Ob1">;
 def _SLASH_Ob2 : CLIgnoredFlag<"Ob2">;
+def _SLASH_Og : CLIgnoredFlag<"Og">;
 def _SLASH_openmp_ : CLIgnoredFlag<"openmp-">;
 def _SLASH_RTC : CLIgnoredJoined<"RTC">;
 def _SLASH_sdl : CLIgnoredFlag<"sdl">;

clang/lib/Driver/MSVCToolChain.cpp

@@ -528,3 +528,101 @@ SanitizerMask MSVCToolChain::getSupportedSanitizers() const {
   Res |= SanitizerKind::Address;
   return Res;
 }
+
+llvm::opt::DerivedArgList *
+MSVCToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args,
+                             const char *BoundArch) const {
+  DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs());
+  const OptTable &Opts = getDriver().getOpts();
+
+  // The -O[12xd] flag actually expands to several flags.  We must desugar the
+  // flags so that options embedded can be negated.  For example, the '-O2' flag
+  // enables '-Oy'.  Expanding '-O2' into its constituent flags allows us to
+  // correctly handle '-O2 -Oy-' where the trailing '-Oy-' disables a single
+  // aspect of '-O2'.
+  //
+  // Note that this expansion logic only applies to the *last* of '[12xd]'.
+
+  // First step is to search for the character we'd like to expand.
+  const char *ExpandChar = nullptr;
+  for (Arg *A : Args) {
+    if (!A->getOption().matches(options::OPT__SLASH_O))
+      continue;
+    StringRef OptStr = A->getValue();
+    for (size_t I = 0, E = OptStr.size(); I != E; ++I) {
+      const char &OptChar = *(OptStr.data() + I);
+      if (OptChar == '1' || OptChar == '2' || OptChar == 'x' || OptChar == 'd')
+        ExpandChar = OptStr.data() + I;
+    }
+  }
+
+  // The -O flag actually takes an amalgam of other options.  For example,
+  // '/Ogyb2' is equivalent to '/Og' '/Oy' '/Ob2'.
+  for (Arg *A : Args) {
+    if (!A->getOption().matches(options::OPT__SLASH_O)) {
+      DAL->append(A);
+      continue;
+    }
+
+    StringRef OptStr = A->getValue();
+    for (size_t I = 0, E = OptStr.size(); I != E; ++I) {
+      const char &OptChar = *(OptStr.data() + I);
+      switch (OptChar) {
+      default:
+        break;
+      case '1':
+      case '2':
+      case 'x':
+      case 'd':
+        if (&OptChar == ExpandChar) {
+          if (OptChar == 'd') {
+            DAL->AddFlagArg(A, Opts.getOption(options::OPT_O0));
+          } else {
+            if (OptChar == '1') {
+              DAL->AddJoinedArg(A, Opts.getOption(options::OPT_O), "s");
+            } else if (OptChar == '2' || OptChar == 'x') {
+              DAL->AddFlagArg(A, Opts.getOption(options::OPT_fbuiltin));
+              DAL->AddJoinedArg(A, Opts.getOption(options::OPT_O), "2");
+            }
+            DAL->AddFlagArg(A,
+                            Opts.getOption(options::OPT_fomit_frame_pointer));
+            if (OptChar == '1' || OptChar == '2')
+              DAL->AddFlagArg(A,
+                              Opts.getOption(options::OPT_ffunction_sections));
+          }
+        }
+        break;
+      case 'b':
+        if (isdigit(OptStr[I + 1]))
+          ++I;
+        break;
+      case 'g':
+        break;
+      case 'i':
+        if (OptStr[I + 1] == '-') {
+          ++I;
+          DAL->AddFlagArg(A, Opts.getOption(options::OPT_fno_builtin));
+        } else {
+          DAL->AddFlagArg(A, Opts.getOption(options::OPT_fbuiltin));
+        }
+        break;
+      case 's':
+        DAL->AddJoinedArg(A, Opts.getOption(options::OPT_O), "s");
+        break;
+      case 't':
+        DAL->AddJoinedArg(A, Opts.getOption(options::OPT_O), "2");
+        break;
+      case 'y':
+        if (OptStr[I + 1] == '-') {
+          ++I;
+          DAL->AddFlagArg(A,
+                          Opts.getOption(options::OPT_fno_omit_frame_pointer));
+        } else {
+          DAL->AddFlagArg(A, Opts.getOption(options::OPT_fomit_frame_pointer));
+        }
+        break;
+      }
+    }
+  }
+  return DAL;
+}

clang/lib/Driver/ToolChains.h

@@ -811,6 +811,10 @@ class LLVM_LIBRARY_VISIBILITY MSVCToolChain : public ToolChain {
   MSVCToolChain(const Driver &D, const llvm::Triple &Triple,
                 const llvm::opt::ArgList &Args);
 
+  llvm::opt::DerivedArgList *
+  TranslateArgs(const llvm::opt::DerivedArgList &Args,
+                const char *BoundArch) const override;
+
   bool IsIntegratedAssemblerDefault() const override;
   bool IsUnwindTablesDefault() const override;
   bool isPICDefault() const override;

clang/lib/Driver/Tools.cpp

@@ -8828,17 +8828,31 @@ std::unique_ptr<Command> visualstudio::Compiler::GetCommand(
   Args.AddAllArgs(CmdArgs, options::OPT_I);
 
   // Optimization level.
+  if (Arg *A = Args.getLastArg(options::OPT_fbuiltin, options::OPT_fno_builtin))
+    CmdArgs.push_back(A->getOption().getID() == options::OPT_fbuiltin ? "/Oi"
+                                                                      : "/Oi-");
   if (Arg *A = Args.getLastArg(options::OPT_O, options::OPT_O0)) {
     if (A->getOption().getID() == options::OPT_O0) {
       CmdArgs.push_back("/Od");
     } else {
+      CmdArgs.push_back("/Og");
+
       StringRef OptLevel = A->getValue();
-      if (OptLevel == "1" || OptLevel == "2" || OptLevel == "s")
-        A->render(Args, CmdArgs);
-      else if (OptLevel == "3")
-        CmdArgs.push_back("/Ox");
+      if (OptLevel == "s" || OptLevel == "z")
+        CmdArgs.push_back("/Os");
+      else
+        CmdArgs.push_back("/Ot");
+
+      CmdArgs.push_back("/Ob2");
     }
   }
+  if (Arg *A = Args.getLastArg(options::OPT_fomit_frame_pointer,
+                               options::OPT_fno_omit_frame_pointer))
+    CmdArgs.push_back(A->getOption().getID() == options::OPT_fomit_frame_pointer
+                          ? "/Oy"
+                          : "/Oy-");
+  if (!Args.hasArg(options::OPT_fwritable_strings))
+    CmdArgs.push_back("/GF");
 
   // Flags for which clang-cl has an alias.
   // FIXME: How can we ensure this stays in sync with relevant clang-cl options?

clang/test/Driver/cl-fallback.c

@@ -14,7 +14,12 @@
 // CHECK: "-D" "foo=bar"
 // CHECK: "-U" "baz"
 // CHECK: "-I" "foo"
-// CHECK: "/Ox"
+// CHECK: "/Oi"
+// CHECK: "/Og"
+// CHECK: "/Ot"
+// CHECK: "/Ob2"
+// CHECK: "/Oy"
+// CHECK: "/GF"
 // CHECK: "/GR-"
 // CHECK: "/Gy-"
 // CHECK: "/Gw-"
@@ -38,16 +43,16 @@
 // O0: "/Od"
 // RUN: %clang_cl /fallback /O1 -### -- %s 2>&1 | FileCheck -check-prefix=O1 %s
 // O1: cl.exe
-// O1: "-O1"
+// O1: "/Og" "/Os" "/Ob2" "/Oy" "/GF" "/Gy"
 // RUN: %clang_cl /fallback /O2 -### -- %s 2>&1 | FileCheck -check-prefix=O2 %s
 // O2: cl.exe
-// O2: "-O2"
+// O2: "/Oi" "/Og" "/Ot" "/Ob2" "/Oy" "/GF" "/Gy"
 // RUN: %clang_cl /fallback /Os -### -- %s 2>&1 | FileCheck -check-prefix=Os %s
 // Os: cl.exe
-// Os: "-Os"
+// Os: "/Os"
 // RUN: %clang_cl /fallback /Ox -### -- %s 2>&1 | FileCheck -check-prefix=Ox %s
 // Ox: cl.exe
-// Ox: "/Ox"
+// Ox: "/Oi" "/Og" "/Ot" "/Ob2" "/Oy" "/GF"
 
 // Only fall back when actually compiling, not for e.g. /P (preprocess).
 // RUN: %clang_cl /fallback /P -### -- %s 2>&1 | FileCheck -check-prefix=P %s

clang/test/Driver/cl-options.c

@@ -81,7 +81,7 @@
 // J: -fno-signed-char
 
 // RUN: %clang_cl /Ofoo -### -- %s 2>&1 | FileCheck -check-prefix=O %s
-// O: -Ofoo
+// O: /Ofoo
 
 // RUN: %clang_cl /Ob0 -### -- %s 2>&1 | FileCheck -check-prefix=Ob0 %s
 // Ob0: -fno-inline
@@ -96,13 +96,24 @@
 // Oi_: -fno-builtin
 
 // RUN: %clang_cl /Os -### -- %s 2>&1 | FileCheck -check-prefix=Os %s
+// Os-NOT: -mdisable-fp-elim
+// Os: -momit-leaf-frame-pointer
 // Os: -Os
 
 // RUN: %clang_cl /Ot -### -- %s 2>&1 | FileCheck -check-prefix=Ot %s
+// Ot-NOT: -mdisable-fp-elim
+// Ot: -momit-leaf-frame-pointer
 // Ot: -O2
 
 // RUN: %clang_cl /Ox -### -- %s 2>&1 | FileCheck -check-prefix=Ox %s
-// Ox: -O3
+// Ox-NOT: -mdisable-fp-elim
+// Ox: -momit-leaf-frame-pointer
+// Ox: -O2
+
+// RUN: %clang_cl /O2sy- -### -- %s 2>&1 | FileCheck -check-prefix=PR24003 %s
+// PR24003: -mdisable-fp-elim
+// PR24003: -momit-leaf-frame-pointer
+// PR24003: -Os
 
 // RUN: %clang_cl /Zs /Oy -- %s 2>&1