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 +def mskip_rax_setup : Flag<["-"], "mskip-rax-setup">, Group<m_Group>, Flags<[NoXarchOption]>, + HelpText<"Skip setting up RAX register when passing variable arguments (x86 only)">; def mstackrealign : Flag<["-"], "mstackrealign">, Group<m_Group>, Flags<[CC1Option]>, ---------------- pengfei wrote: > nickdesaulniers wrote: > > pengfei wrote: > > > MaskRay wrote: > > > > nickdesaulniers wrote: > > > > > I think GCC support `-mno-skip-rax-setup` as well. Can you please add > > > > > that (and tests for it) as well? We don't need to actually handle > > > > > it, I think, but we shouldn't warn about the flag being unsupported, > > > > > for example. > > > > Consider `SimpleMFlag` > > > Thanks for the suggestion. I think adding a `-mno-skip-rax-setup` is > > > simply here. > > Via @MaskRay 's suggestion, can we do something like: > > > > ``` > > def skip_rax_setup : SimpleMFlag<"skip-rax-setup", "Skip", "Do not skip", > > "setting up RAX register when passing variable arguments (x86 only)">; > > ``` > > > > Do we test that this flag is warned about being unused for non-x86 targets? > > (Is it worth adding such a test to clang)? > Another reason I didn't use `SimpleMFlag` is GCC doesn't have help test for > `-mno-skip-rax-setup`. I deliberately hide it in this way. > > > Do we test that this flag is warned about being unused for non-x86 targets? > > (Is it worth adding such a test to clang)? > > I tested it locally. Driver will warn it for other target, Clang doesn't. I > think it should be enough. I don't know if there's precedent, but adding an > irrelevant option test to other target's test folder looks a bit weird to me. > Another reason I didn't use SimpleMFlag is GCC doesn't have help test for > -mno-skip-rax-setup. I deliberately hide it in this way. While clang does try hard to emulate GCC, both in features and interface, that's trying too hard to emulate poor interface. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112413/new/ https://reviews.llvm.org/D112413 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
