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; };
----------------
aaron.ballman wrote:
> tbaeder wrote:
> > 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?
> Let's avoid serializing the floats to strings so that we can parse the string 
> to turn it back into a float later; that's going to have poor performance 
> even if we do get all the corner cases correct regarding things like 
> rounding, etc.
> 
> `APFloat` does not have any sort of serialization functionality beyond 
> through strings representing the value. I think you'd have to invent such an 
> interface.
Do you know who I might talk to regrading such an interface, both the 
implementation as well as general feasibility?


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

Reply via email to