[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-03-01 Thread Ruslan Nikolaev via Phabricator via cfe-commits
nruslan added a comment. @aemerson : yes, it is just part of MS ABI Repository: rC Clang https://reviews.llvm.org/D43108 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-03-01 Thread Amara Emerson via Phabricator via cfe-commits
aemerson added a comment. In https://reviews.llvm.org/D43108#1023300, @nruslan wrote: > By default, stack probes are enabled (i.e., -mstack-arg-probe is the default > behavior) and have the size of 4K in x86. This part what I wanted to clarify, `-mstack-probe-arg` is enabling stack probes if

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-28 Thread Ruslan Nikolaev via Phabricator via cfe-commits
nruslan added a comment. @aemerson : I am not 100% sure, but I think you are talking about different flag. This commit is for supporting a flag to disable stack probes (identical flag can be found in MinGW), it basically only applies to PE/COFF (Windows) targets. It can be useful to compile

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-28 Thread Amara Emerson via Phabricator via cfe-commits
aemerson added a comment. Can we clarify the meaning of this option a bit. The doc you've added here is saying that `-mno-stack-arg-probe` disables stack probes. Then what does `-mstack-arg-probe` mean specifically? Does it mean that only stack probes for ABI required reasons are enabled, or

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-23 Thread Hans Wennborg via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC325901: Support for the mno-stack-arg-probe flag (authored by hans, committed by ). Changed prior to commit: https://reviews.llvm.org/D43108?vs=134377=135631#toc Repository: rC Clang

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-22 Thread Ruslan Nikolaev via Phabricator via cfe-commits
nruslan added a comment. @hans, @craig.topper, @MatzeB: Can someone commit https://reviews.llvm.org/D43108 (this) and https://reviews.llvm.org/D43107 on my behalf? I do not think, I have commit access. https://reviews.llvm.org/D43108 ___

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-16 Thread Hans Wennborg via Phabricator via cfe-commits
hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. This looks good to me. https://reviews.llvm.org/D43108 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-14 Thread Ruslan Nikolaev via Phabricator via cfe-commits
nruslan added a comment. @hans: I responded to the comments and also updated diff for clang with new changes. https://reviews.llvm.org/D43108 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-14 Thread Ruslan Nikolaev via Phabricator via cfe-commits
nruslan added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:4038 + if (Args.hasArg(options::OPT_mno_stack_arg_probe)) +CmdArgs.push_back("-mno-stack-arg-probe"); + hans wrote: > I think you can just do > > ``` > Args.AddLastArg(CmdArgs,

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-14 Thread Ruslan Nikolaev via Phabricator via cfe-commits
nruslan updated this revision to Diff 134377. nruslan marked 3 inline comments as done. nruslan added a comment. Added mstack-arg-probe, tests, and changes related to other comments of the reviewers https://reviews.llvm.org/D43108 Files: docs/ClangCommandLineReference.rst

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-13 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. In https://reviews.llvm.org/D43108#1005904, @nruslan wrote: > @hans: One real-world example is when it is used to compile UEFI code using > PE/COFF targets natively. Obviously, UEFI uses ABI which is basically almost > the same as MS ABI, except that chkstk is not used.

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-12 Thread Ruslan Nikolaev via Phabricator via cfe-commits
nruslan added a comment. @hans: One real-world example is when it is used to compile UEFI code using PE/COFF targets natively. Obviously, UEFI uses ABI which is basically almost the same as MS ABI, except that chkstk is not used. It mostly works (I actually was able to get it running) except

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-12 Thread Matthias Braun via Phabricator via cfe-commits
MatzeB added a comment. - No test. - What about `-mstack-arg-probe`, shouldn't we have that for consistency? - I'd prefer not to review clang changes myself as I don't know that part of the code too well. Repository: rC Clang https://reviews.llvm.org/D43108

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-12 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. I see in the PR that matches a MinGW flag, but I'm curious to the motivation here. In what scenario would the user want to use this, i.e. how do they know it's safe to drop the probes? Comment at: lib/CodeGen/TargetInfo.cpp:2358 -static void

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-11 Thread Ruslan Nikolaev via Phabricator via cfe-commits
nruslan added a comment. @craig.topper, @MatzeB, @hans: Can someone take a look at this change? Repository: rC Clang https://reviews.llvm.org/D43108 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43108: Support for the mno-stack-arg-probe flag

2018-02-08 Thread Ruslan Nikolaev via Phabricator via cfe-commits
nruslan created this revision. nruslan added a reviewer: clang. Herald added a subscriber: cfe-commits. Adds support for this flag. There is also another piece for llvm (separate review). More info: https://bugs.llvm.org/show_bug.cgi?id=36221 Repository: rC Clang