[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-11-17 Thread Phoebe Wang via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGde34a940ae72: [X86] Add -mskip-rax-setup support to align with GCC (authored by pengfei). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112413/new/ https://

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-11-17 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added a comment. Thanks @nickdesaulniers for the review! I found another problem with `SimpleMFlag`, to which `MarshallingInfoFlag` cannot be assigned separately. I'll land it as is then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11241

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-11-17 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Thanks for the feature. I'd still prefer to see `SimpleMFlag` used, but it's not a big deal to me. Comment at: clang/include/clang/Driver/Options.td:3193

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-11-17 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added a comment. > It might be nice to add an llvm-link test that produces an error when this > module metadata doesn't match. Basically, it would be nice to error if during > LTO we link together two modules that differ in their use of -mskip-rax-setup > since that has ABI implications

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-11-17 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei updated this revision to Diff 387874. pengfei marked 3 inline comments as done. pengfei added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112413/new/ https://reviews.llvm.org/D112413 Files: clang/inc

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-11-16 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: llvm/test/CodeGen/X86/pr23258.ll:22 + +declare dso_local void @bar(i32, ...) + nickdesaulniers wrote: > Is there anything one the callee side that we should check? I assume the > callee does some check of `%al`

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-11-16 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. It might be nice to add an `llvm-link` test that produces an error when this module metadata doesn't match. Basically, it would be nice to error if during LTO we link together two modules that differ in their use of `-mskip-rax-setup` since that has ABI implica

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-11-14 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added inline comments. Comment at: clang/include/clang/Driver/Options.td:3193 +def mskip_rax_setup : Flag<["-"], "mskip-rax-setup">, Group, Flags<[NoXarchOption]>, + HelpText<"Skip setting up RAX register when passing variable arguments (x86 only)">; def mstackrealign

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-11-14 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei updated this revision to Diff 387147. pengfei marked 3 inline comments as done. pengfei added a comment. Herald added a subscriber: dexonsmith. Address review comments. 1. Add support for `-mno-skip-rax-setup` and its test; 2. Use module flag metadata instead of command line option in bac

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-10-27 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:96 +static cl::opt SkipRaxSetup( +"x86-skip-rax-setup", cl::init(false), MaskRay wrote: > nickdesaulniers wrote: > > If it's a command line option rather than encoded

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-10-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:3193 +def mskip_rax_setup : Flag<["-"], "mskip-rax-setup">, Group, Flags<[NoXarchOption]>, + HelpText<"Skip setting up RAX register when passing variable arguments (x86 only)">; def mstackrealign

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-10-27 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers requested changes to this revision. nickdesaulniers added a comment. This revision now requires changes to proceed. Thanks for the patch; I'd like to see this encoded in the IR (perhaps as a function level attribute), the `-mno-` variant added, and perhaps help the user if they f

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-10-26 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. In D112413#3089073 , @nickdesaulniers wrote: > Fixes https://bugs.llvm.org/show_bug.cgi?id=23258? sorry, I missed the PR # in the description; my mistake. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-10-26 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added a comment. In D112413#3089073 , @nickdesaulniers wrote: > Fixes https://bugs.llvm.org/show_bug.cgi?id=23258? Yes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112413/new/ https://reviews.l

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-10-26 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. Fixes https://bugs.llvm.org/show_bug.cgi?id=23258? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112413/new/ https://reviews.llvm.org/D112413 ___ cfe-commits mailing list

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-10-25 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added inline comments. Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:99 +cl::desc("Skips setting up the RAX register when SSE is disabled and there " + "are no variable arguments passed in vector registers."), +cl::Hidden); joer

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-10-25 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei updated this revision to Diff 381997. pengfei added a comment. Address review comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112413/new/ https://reviews.llvm.org/D112413 Files: clang/include/clang/Driver/Options.td clang/lib/Dr

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-10-25 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added inline comments. Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:99 +cl::desc("Skips setting up the RAX register when SSE is disabled and there " + "are no variable arguments passed in vector registers."), +cl::Hidden); The de

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

2021-10-25 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei created this revision. pengfei added reviewers: MaskRay, hjl.tools, erichkeane, LuoYuanke, craig.topper. Herald added subscribers: dang, hiraditya. pengfei requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. This fixe