[PATCH] D146441: [APFloat] Add E4M3B11FNUZ

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/MicrosoftMangle.cpp:848
   case APFloat::S_Float8E4M3FNUZ:
+  case APFloat::S_Float8E4M3B11FNUZ:
     llvm_unreachable("Tried to mangle unexpected APFloat semantics");
----------------
majnemer wrote:
> aaron.ballman wrote:
> > Why are there no changes needed for the Itanium mangler? And when did we start adding a bunch of esoteric float formats? Was there an RFC for this that I missed? (If I did miss something, then sorry for the noise.)
> Hi Aaron,
> 
> The Itanium mangler doesn't depend on the APFloat's semantics, it just depends on:
> - The Clang AST type of the float.
> - The bit pattern of the APFloat.
> 
> The code to handle the bit pattern is written generically, regardless of APFloat semantics.
> Because of this and the lack of a new Clang built-in floating type results in no necessary changes to the Itanium mangler.
> 
> This just comes down to a quirk in how the MSVC and Itanium manglers are written: new APFloat semantics necessarily result in the MSVC mangler "noticing" while they don't for the Itanium mangler.
> 
> As to your question about an RFC, one was posted for some similar but different formats that people are interested in [here](https://discourse.llvm.org/t/rfc-adding-the-amd-graphcore-maybe-others-float8-formats-to-apfloat/67969)
> 
> The general consensus on that RFC was that APFloat should support formats that people use in practice. This format is incorporated as part of real hardware in the real world as well as an [open source compiler framework](https://github.com/openxla/stablehlo/blob/main/rfcs/20230309-e4m3b11.md).
Thank you for the explanation (and IRC discussion)! This all seems reasonable to me. I was mostly worried that there were more Clang parts that needed attention and discussion, but this is more used for backend work and isn't really exposed to the frontend except in these sort of fully covered switch cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146441