[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag
David Spickett via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 30 01:59:50 PDT 2022
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 needed)
Lack of docs right now is fine, we have enough to know this exists and it's quite simple.
With my one final comment addressed, this LGTM.
================
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)
----------------
DavidSpickett wrote:
> This would read better to me if you put them all in one and put the most important check first like:
> ```
> if ( UArgs->hasArg(options::OPT__SLASH_arm64EC) &&
> ( (TC.getTriple().getArch() != llvm::Triple::aarch64 ||
> TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec)) {
> ```
This is good but I didn't emphasise one bit. Putting the arm64ec option check first saves reading the rest if the reader knows it's not relevant.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134788/new/
https://reviews.llvm.org/D134788
More information about the cfe-commits
mailing list