This revision was automatically updated to reflect the committed changes.
Closed by commit rC342977: [clang-cl] Provide separate flags for all the /O
variants (authored by hans, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52266?vs=166862=166884#toc
Repository:
rC
hans added a comment.
In https://reviews.llvm.org/D52266#1244733, @thakis wrote:
> > Actually, trying this out with MSVC, I don't see any __chkstk calls with
> > /https://reviews.llvm.org/owners/package/1/, or with eg. /Gs1 for that
> > matter:
>
> (.guess the bug should also cover to
thakis added a comment.
> Actually, trying this out with MSVC, I don't see any __chkstk calls with
> /https://reviews.llvm.org/owners/package/1/, or with eg. /Gs1 for that matter:
(.guess the bug should also cover to eventually not alias /Gs to /Gs0 and/or
/Gs to -mstack-probe-size either if
thakis accepted this revision.
thakis added a comment.
This revision is now accepted and ready to land.
lgtm, thanks!
Maybe file a bug on figuring out the /Gs story and add a FIXME linking to it.
Weird.
Comment at: include/clang/Driver/CLCompatOptions.td:118
+def _SLASH_O :
hans updated this revision to Diff 166862.
hans added a comment.
Addressing all comments.
https://reviews.llvm.org/D52266
Files:
include/clang/Driver/CLCompatOptions.td
lib/Driver/ToolChains/MSVC.cpp
test/Driver/Xarch.c
Index: test/Driver/Xarch.c
hans marked 7 inline comments as done.
hans added a comment.
> 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
thakis added inline comments.
Comment at: include/clang/Driver/CLCompatOptions.td:121
+def : CLFlag<"Ob0">, Alias<_SLASH_O>, AliasArgs<["b0"]>, HelpText<"Disable
function inlining">;
+def : CLFlag<"Ob1">, Alias<_SLASH_O>, AliasArgs<["b1"]>, HelpText<"Only inline
functions
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
hans updated this revision to Diff 166252.
hans added a comment.
Uploading new diff.
https://reviews.llvm.org/D52266
Files:
include/clang/Driver/CLCompatOptions.td
lib/Driver/ToolChains/MSVC.cpp
test/Driver/Xarch.c
Index: test/Driver/Xarch.c
hans added a comment.
Sorry, I didn't realize we both set off to do this in parallel. I've
incorporated your changes into my patch.
Comment at: test/Driver/Xarch.c:5
+// RUN: not grep ' "-O3" ' %t.log
+// RUN: grep "argument unused during compilation: '-Xarch_i386 -O3'"
thakis added a comment.
FWIW the recommendation against /Ox in my version is because of
https://github.com/ulfjack/ryu/pull/70#issuecomment-412168459
https://reviews.llvm.org/D52266
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
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-"]>,
compnerd added inline comments.
Comment at: test/Driver/Xarch.c:5
+// RUN: not grep ' "-O3" ' %t.log
+// RUN: grep "argument unused during compilation: '-Xarch_i386 -O3'" %t.log
// RUN: not %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -o -Xarch_i386
-S %s -S -Xarch_i386
hans created this revision.
hans added reviewers: thakis, rnk.
This provides better help text in "clang-cl /?".
Also it cleans things up a bit: previously "/Od" could be handled either as a
separate flag aliased to "-O0", or by the main optimization flag processing in
TranslateOptArg. With
14 matches
Mail list logo
