stuij 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 + ---------------- chill wrote: > 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. Just as a house-keeping note: If we would change the naming, I think we can all agree that this ticket itself shouldn't be the place where we want to do this. I'm happy for the conversation to carry on here, but I think we can move the ticket forward at the same time. ================ Comment at: llvm/test/CodeGen/AArch64/aarch64-bf16-ldst-intrinsics.ll:916 + +attributes #0 = { norecurse nounwind readonly "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="64" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+bf16,+neon" "unsafe-fp-math"="false" "use-soft-float"="false" } +attributes #1 = { norecurse nounwind readonly "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="128" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+bf16,+neon" "unsafe-fp-math"="false" "use-soft-float"="false" } ---------------- You should be able to do without all these big blocks of attributes which I guess were generated from C -> IR conversion. Just remove it and the `#x`s after the function declarations (maybe replace them with `nounwind`). 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