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

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

Reply via email to