[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

Thu Apr 16 07:16:10 PDT 2020

tambre added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64.td:607
+                                   FeatureSVE
+                                   ]>;
+
----------------
sdesmalen wrote:
> Not sure how accurate the link you referenced is, but it doesn't mention SVE or Crypto extensions. Is that an omission in the blog post?
All the ARM vector extensions are a bit confusing. SVE indeed isn't supported, I've removed it.
I confirmed that AES, SHA1 and SHA2 extensions work on a real machine.

================
Comment at: llvm/unittests/Support/Host.cpp:271
+BogoMIPS        : 62.50
+Features        : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm dcpop
+CPU implementer : 0x4e
----------------
sdesmalen wrote:
> nit: it probably doesn't matter much for the function `getHostCPUNameForARM`, but the feature list seems incomplete.
The output is from a Xavier system running the Linux variant provided by Nvidia.

================
Comment at: llvm/unittests/Support/TargetParserTest.cpp:980
+      AArch64::AEK_CRC | AArch64::AEK_CRYPTO | AArch64::AEK_FP |
+          AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
+          AArch64::AEK_LSE | AArch64::AEK_SVE | AArch64::AEK_RDM,
----------------
sdesmalen wrote:
> nit: odd indentation here, have you used clang-format?
Fixed manually. Using clang-format results in unpleasant formatting similar to "apple-a10" and "apple-a11" above.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77940