[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

Nico Weber via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 19 08:10:29 PDT 2018


thakis added inline comments.


================
Comment at: include/clang/Driver/CLCompatOptions.td:129
+def _SLASH_Oy : CLFlag<"Oy">, Alias<_SLASH_O>, AliasArgs<["y"]>, HelpText<"Enable frame pointer omission (x86 only)">;
+def _SLASH_Oy_ : CLFlag<"Oy-">, Alias<_SLASH_O>, AliasArgs<["y-"]>, HelpText<"Disable frame pointer omission (x86 only)">;
+
----------------
I took a stab at this myself for a bit, and did a few things differently (but most the same way).

Here's how they /? output looks in my local binary:

```
  /O1                     Minimize size, equivalent to /Og /Os /Oy /Ob2 /Gs /GF /Gy
  /O2                     Maximize speed, equivalent to /Og /Oi /Ot /Oy /Ob2 /Gs /GF /Gy
  /Ob0                    Disable function inlining
  /Ob1                    Only inline functions explicitly or implicitly marked inline
  /Ob2                    Allow compiler to inline all functions it deems beneficial
  /Od                     Disable optimization
  /Og                     No effect
  /Oi-                    Disable use of builtin functions
  /Oi                     Enable use of builtin functions
  /Os                     Optimize for size
  /Ot                     Optimize for speed
  /Ox                     Deprecated, use O2 instead; equivalent to /Og /Oi /Ot /Oy /Ob2
  /Oy-                    Don't omit frame pointers (default)
  /Oy                     Omit frame pointers (x86 only)
  /O<value>               Set several /O flags at once; '/O2y-' is the same as '/O2 /y-' for example
```

Since we reference them now, I also added these:

```
  /GF                     Enable string pooling (default)
...
  /Gs                     Same as /Gs0
  /Gs<value>              Set stack probe size (default 4096)
...
  /Gy-                    Don't put each function in its own section (default)
  /Gy                     Put each function in its own section
```

As far as I can tell we do not make /O1 and /O2 imply /Gs -- we keep it at the default value of 4096 (?) Fixing this probably means increasing chrome's size (?).

cl.exe doesn't support O0; I wonder why rnk added it. I asked on the revision that added it.

The name after def isn't needed; maybe we should omit the names of flags we don't reference?

Here's my local diff, mix and match as you see fit:

```
Index: include/clang/Driver/CLCompatOptions.td
===================================================================
--- include/clang/Driver/CLCompatOptions.td	(revision 342334)
+++ include/clang/Driver/CLCompatOptions.td	(working copy)
@@ -85,16 +85,21 @@
   HelpText<"Assume thread-local variables are defined in the executable">;
 def _SLASH_GR : CLFlag<"GR">, HelpText<"Enable emission of RTTI data">;
 def _SLASH_GR_ : CLFlag<"GR-">, HelpText<"Disable emission of RTTI data">;
+def _SLASH_GF : CLIgnoredFlag<"GF">,
+  HelpText<"Enable string pooling (default)">;
 def _SLASH_GF_ : CLFlag<"GF-">, HelpText<"Disable string pooling">,
   Alias<fwritable_strings>;
-def _SLASH_GS : CLFlag<"GS">, HelpText<"Enable buffer security check">;
+def _SLASH_GS : CLFlag<"GS">,
+  HelpText<"Enable buffer security check (default)">;
 def _SLASH_GS_ : CLFlag<"GS-">, HelpText<"Disable buffer security check">;
-def _SLASH_Gs : CLJoined<"Gs">, HelpText<"Set stack probe size">,
+def: CLFlag<"Gs">, HelpText<"Same as /Gs0">,
+  Alias<mstack_probe_size>, AliasArgs<["0"]>;
+def _SLASH_Gs : CLJoined<"Gs">, HelpText<"Set stack probe size (default 4096)">,
   Alias<mstack_probe_size>;
 def _SLASH_Gy : CLFlag<"Gy">, HelpText<"Put each function in its own section">,
   Alias<ffunction_sections>;
 def _SLASH_Gy_ : CLFlag<"Gy-">,
-  HelpText<"Don't put each function in its own section">,
+  HelpText<"Don't put each function in its own section (default)">,
   Alias<fno_function_sections>;
 def _SLASH_Gw : CLFlag<"Gw">, HelpText<"Put each data item in its own section">,
   Alias<fdata_sections>;
@@ -109,18 +114,50 @@
   Alias<I>;
 def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,
   Alias<funsigned_char>;
+
+// FIXME: cl.exe doesn't understand this, not sure why clang-cl does.
 def _SLASH_O0 : CLFlag<"O0">, Alias<O0>;
-// /Oy- is handled by the /O option because /Oy- only has an effect on 32-bit.
-def _SLASH_O : CLJoined<"O">, HelpText<"Optimization level">;
-def _SLASH_Od : CLFlag<"Od">, HelpText<"Disable optimization">, Alias<O0>;
-def _SLASH_Oi : CLFlag<"Oi">, HelpText<"Enable use of builtin functions">,
-  Alias<fbuiltin>;
-def _SLASH_Oi_ : CLFlag<"Oi-">, HelpText<"Disable use of builtin functions">,
-  Alias<fno_builtin>;
-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_O : CLJoined<"O">,
+  HelpText<"Set several /O flags at once; "
+           "'/O2y-' is the same as '/O2 /y-' for example">;
+
+// FIXME: Looks like this doesn't actually imply /Gs in clang-cl yet.
+def _SLASH_O1 : CLFlag<"O1">,
+  HelpText<"Minimize size, equivalent to /Og /Os /Oy /Ob2 /Gs /GF /Gy">,
+  Alias<_SLASH_O>, AliasArgs<["1"]>;
+def _SLASH_O2 : CLFlag<"O2">,
+  HelpText<"Maximize speed, equivalent to /Og /Oi /Ot /Oy /Ob2 /Gs /GF /Gy">,
+  Alias<_SLASH_O>, AliasArgs<["2"]>;
+def _SLASH_Ox : CLFlag<"Ox">,
+  HelpText<"Deprecated, use O2 instead; equivalent to /Og /Oi /Ot /Oy /Ob2">,
+  Alias<_SLASH_O>, AliasArgs<["x"]>;
+def _SLASH_Od : CLFlag<"Od">, HelpText<"Disable optimization">,
+  Alias<_SLASH_O>, AliasArgs<["d"]>;
+
+def: CLFlag<"Ob0">, HelpText<"Disable function inlining">,
+  Alias<_SLASH_O>, AliasArgs<["b0"]>;
+def: CLFlag<"Ob1">,
+  HelpText<"Only inline functions explicitly or implicitly marked inline">,
+  Alias<_SLASH_O>, AliasArgs<["b1"]>;
+def: CLFlag<"Ob2">,
+  HelpText<"Allow compiler to inline all functions it deems beneficial">,
+  Alias<_SLASH_O>, AliasArgs<["b2"]>;
+def: CLFlag<"Og">, HelpText<"No effect">,
+  Alias<_SLASH_O>, AliasArgs<["g"]>;
+def: CLFlag<"Oi">, HelpText<"Enable use of builtin functions">,
+  Alias<_SLASH_O>, AliasArgs<["i"]>;
+def: CLFlag<"Oi-">, HelpText<"Disable use of builtin functions">,
+  Alias<_SLASH_O>, AliasArgs<["i-"]>;
+def: CLFlag<"Os">, HelpText<"Optimize for size">,
+  Alias<_SLASH_O>, AliasArgs<["s"]>;
+def: CLFlag<"Ot">, HelpText<"Optimize for speed">,
+  Alias<_SLASH_O>, AliasArgs<["t"]>;
+def: CLFlag<"Oy">, HelpText<"Omit frame pointers (x86 only)">,
+  Alias<_SLASH_O>, AliasArgs<["y"]>;
+def: CLFlag<"Oy-">, HelpText<"Don't omit frame pointers (default)">,
+  Alias<_SLASH_O>, AliasArgs<["y-"]>;
+
 def _SLASH_QUESTION : CLFlag<"?">, Alias<help>,
   HelpText<"Display available options">;
 def _SLASH_Qvec : CLFlag<"Qvec">,
@@ -326,10 +363,8 @@
 def _SLASH_FC : CLIgnoredFlag<"FC">;
 def _SLASH_Fd : CLIgnoredJoined<"Fd">;
 def _SLASH_FS : CLIgnoredFlag<"FS">;
-def _SLASH_GF : CLIgnoredFlag<"GF">;
 def _SLASH_kernel_ : CLIgnoredFlag<"kernel-">;
 def _SLASH_nologo : CLIgnoredFlag<"nologo">;
-def _SLASH_Og : CLIgnoredFlag<"Og">;
 def _SLASH_openmp_ : CLIgnoredFlag<"openmp-">;
 def _SLASH_permissive_ : CLIgnoredFlag<"permissive-">;
 def _SLASH_RTC : CLIgnoredJoined<"RTC">;
```


https://reviews.llvm.org/D52266





More information about the cfe-commits mailing list