thakis added a comment. In https://reviews.llvm.org/D52266#1240304, @hans wrote:
> Sorry, I didn't realize we both set off to do this in parallel. I've > incorporated your changes into my patch. No worries, I didn't do anything I wouldn't have done for reviewing this :-) Thoughts on "As far as I can tell we do not make /https://reviews.llvm.org/owners/package/1/ and /https://reviews.llvm.org/owners/package/2/ imply /Gs -- we keep it at the default value of 4096 (?) Fixing this probably means increasing chrome's size (?)."? ================ Comment at: include/clang/Driver/CLCompatOptions.td:94 +def : CLFlag<"Gs">, HelpText<"Same as /Gs0">, Alias<mstack_probe_size>, AliasArgs<["0"]>; +def _SLASH_Gs : CLJoined<"Gs">, HelpText<"Set stack probe size">, Alias<mstack_probe_size>; def _SLASH_Gy : CLFlag<"Gy">, HelpText<"Put each function in its own section">, ---------------- Want to add "(default 4096)" here? ================ Comment at: include/clang/Driver/CLCompatOptions.td:115 +// The _SLASH_O option handles all the /O flags, but we also provide separate aliased options to provide separate help messages. +// FIXME: Not sure why we have -O0 here; MSVC doesn't support that. +def _SLASH_O : CLJoined<"O">, HelpText<"Set several /O flags at once; e.g. '/O2y-' is the same as '/O2 /y-'">, MetaVarName<"<flags>">; ---------------- Move FIXME down one so it's next to the O0 flag? ================ Comment at: include/clang/Driver/CLCompatOptions.td:118 +def : CLFlag<"O0">, Alias<O0>, HelpText<"Disable optimization">; +def : CLFlag<"O1">, Alias<_SLASH_O>, AliasArgs<["1"]>, HelpText<"Optimize for small size (equivalent to /Og /Os /Oy /Ob2 /Gs /GF /Gy)">; +def : CLFlag<"O2">, Alias<_SLASH_O>, AliasArgs<["2"]>, HelpText<"Optimize for maximum speed (equivalent to /Og /Oi /Ot /Oy /Ob2 /Gs /GF /Gy)">; ---------------- Nit: Just "optimize for size" / "optimize for speed" is shorter ================ Comment at: include/clang/Driver/CLCompatOptions.td:123 +def : CLFlag<"Ob2">, Alias<_SLASH_O>, AliasArgs<["b2"]>, HelpText<"Inline functions as deemed beneficial by the compiler">; +def : CLFlag<"Od">, Alias<O0>, HelpText<"Disable optimization">; +def : CLFlag<"Og">, Alias<_SLASH_O>, AliasArgs<["g"]>, HelpText<"No effect">; ---------------- Why not alias this to _SLASH_O, d like the rest? ================ Comment at: include/clang/Driver/CLCompatOptions.td:128 +def : CLFlag<"Os">, Alias<_SLASH_O>, AliasArgs<["s"]>, HelpText<"Optimize for small size over speed">; +def : CLFlag<"Ot">, Alias<_SLASH_O>, AliasArgs<["t"]>, HelpText<"Optimize for speed over small size">; +def : CLFlag<"Ox">, Alias<_SLASH_O>, AliasArgs<["x"]>, HelpText<"Deprecated; use /O2 instead (equivalent to /Og /Oi /Ot /Oy /Ob2)">; ---------------- nit: I liked the old help text for the previous 4 more, it was more concise ================ Comment at: include/clang/Driver/CLCompatOptions.td:129 +def : CLFlag<"Ot">, Alias<_SLASH_O>, AliasArgs<["t"]>, HelpText<"Optimize for speed over small size">; +def : CLFlag<"Ox">, Alias<_SLASH_O>, AliasArgs<["x"]>, HelpText<"Deprecated; use /O2 instead (equivalent to /Og /Oi /Ot /Oy /Ob2)">; +def : CLFlag<"Oy">, Alias<_SLASH_O>, AliasArgs<["y"]>, HelpText<"Enable frame pointer omission (x86 only)">; ---------------- nit: With this punctuation it looks ambiguous to me if the parenthetical refers to /O2 or /Ox. https://reviews.llvm.org/D52266 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
