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) ---------------- DavidSpickett wrote: > 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. I believe the condition `UArgs->hasArg(options::OPT__SLASH_arm64EC)` should be much heavier than `(TC.getTriple().getArch() != llvm::Triple::aarch64 || TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec)`. So I put the Arch and SubArch check first here. I don't understand why we should put the hasArg check first. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134788/new/ https://reviews.llvm.org/D134788 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits