LukeGeeson marked 2 inline comments as done.
LukeGeeson 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:
> 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.


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

https://reviews.llvm.org/D80716



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to