tbaeder added inline comments.
================ Comment at: clang/lib/AST/Interp/Floating.h:27-29 + template <unsigned ReprBits> struct Repr; + template <> struct Repr<32> { using Type = float; }; + template <> struct Repr<64> { using Type = double; }; ---------------- jcranmer-intel wrote: > tbaeder wrote: > > jcranmer-intel wrote: > > > aaron.ballman wrote: > > > > Er, how will this extend to `long double` where the number of bits is > > > > rather more difficult? > > > Or `half` and `bfloat`, which are both 16-bit floating-point types? > > I have spent some time with this today and tried to simply always use > > `APFloat` instead of a primitive type. Unfortunately that doesn't work > > because what we put on the stack is not the `Floating` (or `Integral`), but > > the underlying primitive type. So even if we do the final math (in `::add`, > > etc) via `APFloat`, we need something we can serialize to `char[]` so we > > can put it on the stack. Do you think that would work? > I don't know enough about the structure of the bytecode interpreter here to > say for sure, but this smells to me like you're baking in an assumption that > every primitive target type has a corresponding primitive type on the host. > This assumption just doesn't hold when it comes to floating point (only two > of the seven types, `float` and `double`, are generally portable, and even > then, there be dragons in some corner cases). > > If you do need to continue down this route, there are two requirements that > should be upheld: > * The representation shouldn't assume that the underlying primitive type > exists on host (bfloat16 and float128 are better test cases here). > * Conversion to/from host primitive types shouldn't be easy to accidentally > do. > > (Worth repeating again that bit size is insufficient to distinguish floating > point types: `bfloat` and `half` are both 16-bit, PPC `long double` and IEEE > 754 quad precision are both 128-bit, and x86 `long double` is 80 bits stored > as 96 bits on 32-bit and 128 bits on 64-bit.) Well, is there a way to convert an APFloat to a char[] that would work instead of going to float/double and storing that? The only thing I see in the docs is `convertToHexString()` (and the docs don't mention whether the conversion is lossy). If not, do you think adding such a conversion to `APFloat` and its various implementations is the better way forward? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134859/new/ https://reviews.llvm.org/D134859 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits