[PATCH] D76062: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.


================
Comment at: clang/lib/Basic/Targets/AArch64.cpp:184
+  // Also include the Armv8.5 defines
+  // FIXME: Armv8.6 makes some extensions mandatory. Handle them here.
+  getTargetDefinesARMV85A(Opts, Builder);
----------------
Can you be more specific, what are we missing here?

Hmm, now I see the same above: "FIXME: Armv8.5 makes some extensions mandatory. Handle them here."
While you're at it, can you also change that?


================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:52
+              AArch64::AEK_RDM  | AArch64::AEK_RCPC | AArch64::AEK_DOTPROD |
+              AArch64::AEK_SM4  | AArch64::AEK_SHA3 | AArch64::AEK_BF16    |
+              AArch64::AEK_SHA2 | AArch64::AEK_AES  | AArch64::AEK_I8MM))
----------------
just double checking (because I can't remember): BF16 is a mandatory extension?


================
Comment at: llvm/lib/Target/ARM/ARM.td:532
+                                   "Support ARM v8.6a instructions",
+                                   [HasV8_5aOps, FeatureBF16]>;
+
----------------
it's implied here, so looks mandatory.


================
Comment at: llvm/lib/Target/ARM/ARMSubtarget.h:260
 
+  /// HasBF16 - True if subtarget supports BFloat16 floating point
+  bool HasBF16 = false;
----------------
nit:  BFloat16 floating point ->  BFloat16 floating point operations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76062