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

Reply via email to