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

Fri Jan 20 03:34:31 PST 2023

stuij added inline comments.

================
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))
----------------
ab wrote:
> stuij wrote:
> > SjoerdMeijer wrote:
> > > just double checking (because I can't remember): BF16 is a mandatory extension?
> > for 8.2 it isn't, for 8.6 it is
> Belated question: what's the rationale for replacing `AEK_CRYPTO` with SM4+SHA3+SHA2(+AES)?  I'm not aware of the required crypto bits changing, but maybe I missed something.  There's a related question around how we should deal with crypto here in the first place (remove FK, remove the crypto exts?), but that seems orthogonal to v8.6a+ vs. v8.5a- implying different crypto extensions.
> 
> For context, this comes up in D134351 where specifying `V8_6A` would enable SM4, which we don't support.
Belated (initial) answer: yes unfortunately our crypto story is a bit of a mess (mostly related to it mandatory yes or no). we have a catch-all ticket on our backlog to deal with crypto inconsistencies, which will hopefully be picked up soon.

I've added your point, and so again hopefully soon we can provide clarity and smooth things out.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76062