[PATCH] D80716: [AArch64]: BFloat Load/Store Intrinsics&CodeGen

[Momchil Velikov via Phabricator via cfe-commits]

chill added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/aarch64-bf16-ldst-intrinsics.ll:264
+; Function Attrs: argmemonly nounwind readonly
+declare { <8 x bfloat>, <8 x bfloat> } @llvm.aarch64.neon.ld2lane.v8bf16.p0i8(<8 x bfloat>, <8 x bfloat>, i64, i8*) #3
+
----------------
SjoerdMeijer wrote:
> chill wrote:
> > SjoerdMeijer wrote:
> > > chill wrote:
> > > > LukeGeeson wrote:
> > > > > SjoerdMeijer wrote:
> > > > > > LukeGeeson wrote:
> > > > > > > arsenm wrote:
> > > > > > > > Why is the IR type name bfloat and not bfloat16?
> > > > > > > The naming for the IR type was agreed upon here after quite a big discussion. 
> > > > > > > https://reviews.llvm.org/D78190
> > > > > > I regret very much that I didn't notice this earlier... I.e., I noticed this in D76077 and wrote that I am relatively unhappy about this (I think I mentioned this on another ticket too).
> > > > > > Because like @arsenm , I would expect the IR type name to be bfloat16.
> > > > > > 
> > > > > > Correct me if I am wrong, but I don't see a big discussion about this in D78190. I only see 1 or 2 comments about `BFloat` vs `Bfloat`.
> > > > > I cannot see a discussion about the IR type name per-se but I can see you were both involved in the discussion more generally.
> > > > > 
> > > > > I am concerned that this patch is the wrong place to discuss such issues, and that we should bring this up in a more appropriate place as you mention so that this patch isn't held back.
> > > > I don't see a compelling reason for the name to be `bfloat16` or `bfloat3`, etc. Like other floating-point types (`float`, `double`, and `half`), the name denotes a specific externally defined format, unlike `iN`.
> > > > Like other floating-point types (float, double, and half), the name denotes a specific externally defined format, 
> > > 
> > > Is the defined format not called bfloat16?
> > Indeed, people use the name "bfloat16". But then the `half`, `float`, and `double` also differ from the official `binary16`, `binarty32`, and `binary64`.
> > IMHO `bfloat` fits better in the LLVM IR naming convention.
> yeah, so that's exactly why I don't follow your logic. If there's any logic in the names here, the mapping from source-language type to IR type seems the most plausible one. And I just don't see the benefit of dropping the 16, and how that would fit better in some naming scheme or how that makes things clearer here.
What source language?

That said, I'm resigning from the bikeshedding here.


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

https://reviews.llvm.org/D80716