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

David Spickett via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 29 02:21:30 PDT 2022


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.



================
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)
----------------
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)) {
```


================
Comment at: clang/test/Driver/cl-options.c:787
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck %s --check-prefix ARM64EC
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+
----------------
Add an `ARM64EC-NOT` check for this part to check there is no warning.

Not going to catch much in this change but if someone goes digging into target setting they might change it by other means than `--target` and not realise.


================
Comment at: clang/test/Driver/cl-options.c:790
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -target x86_64-pc-windows-msvc  -### -- %s 2>&1 | FileCheck %s --check-prefix ARM64EC_OVERRIDE
+// ARM64EC_OVERRIDE: warning: /arm64EC has been overridden by specified target:x86_64-pc-windows-msvc; option ignored
+
----------------
A space after the ':' is a bit easier to parse IMO (unless this is matching an msvc warning in which case fair enough keep it as is).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134788/new/

https://reviews.llvm.org/D134788



More information about the cfe-commits mailing list