[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

efriedma added a comment. I think we want to test both that the warning isn't triggered when --target isn't specified, and that the warning is triggered when --target is specified. Comment at: clang/test/Driver/cl-options.c:787 +// ARM64EC: "-triple"

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

bcl5980 added a comment. In D134788#3830987 , @efriedma wrote: > Missing testcase for the case where the warning is triggered? Based on David's comment I remove the test case with specified target. Not going to catch much in this change but if

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

efriedma added a comment. Missing testcase for the case where the warning is triggered? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134788/new/ https://reviews.llvm.org/D134788 ___ cfe-commits mailing

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

This revision was automatically updated to reflect the committed changes. Closed by commit rGb0fff3db6ada: [ARM64EC][clang-cl] Add /arm64EC flag (authored by bcl5980). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134788/new/

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

bcl5980 marked 4 inline comments as done. bcl5980 added a comment. Thanks for the detail explaination. I agree that if the cost is the same hasArg is better to move to the first. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134788/new/ https://reviews.llvm.org/D134788

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

DavidSpickett added a comment. This looks fine as is, all comments addressed as far as I can see. Comment at: clang/lib/Driver/Driver.cpp:1384 + TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) { +if (UArgs->hasArg(options::OPT__SLASH_arm64EC)) { +

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

bcl5980 added inline comments. Comment at: clang/lib/Driver/Driver.cpp:1384 + TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) { +if (UArgs->hasArg(options::OPT__SLASH_arm64EC)) { + getDiags().Report(clang::diag::warn_target_override_arm64ec)

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. Please mark comments as done if you feel that they have been addressed. (I know this can be a bit weird, do you mark them or do I, I go with the former, I can disagree if

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

bcl5980 added a comment. In D134788#3823181 , @DavidSpickett wrote: > Has Microsoft documented this option? Not doubting it exists but I mostly see > how to use it rather than a specific page describing the option and its > behaviour. > > Not a big

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

bcl5980 updated this revision to Diff 464128. bcl5980 added a comment. address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134788/new/ https://reviews.llvm.org/D134788 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clang/Driver/Options.td

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

DavidSpickett added a comment. Has Microsoft documented this option? Not doubting it exists but I mostly see how to use it rather than a specific page describing the option and its behaviour. Not a big deal but if there is one, please include a link to it in the commit message.

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

bcl5980 updated this revision to Diff 463746. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134788/new/ https://reviews.llvm.org/D134788 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clang/Driver/Options.td clang/lib/Driver/Driver.cpp

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

bcl5980 updated this revision to Diff 463744. bcl5980 added a comment. add warning when /arm64EC has been override CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134788/new/ https://reviews.llvm.org/D134788 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

efriedma added a comment. Please add a testcase verifying the interaction between "/arm64EC" and explicitly specifying "--target". (If we end up ignoring the "/arm64EC" flag, we probably want a warning.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION