dblaikie 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; };
----------------
tbaeder wrote:
> dblaikie wrote:
> > jcranmer-intel wrote:
> > > aaron.ballman wrote:
> > > > tbaeder wrote:
> > > > > 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?
> > > > I think there may be at least two ways to do this: use an `APFloat` and
> > > > put the serialization interfaces there, or use an `APValue` and put the
> > > > serialization interfaces there.
> > > >
> > > > Because `APFloat` is an ADT in LLVM, I think it should probably go up
> > > > on Discourse for broader discussion. @chandlerc is still listed as the
> > > > code owner for ADTs but he's not been active in quite some time.
> > > > Instead, I would recommend talking to @dblaikie (he's got a good eye
> > > > for ADT work in general) and @foad, @RKSimon, and @sepavloff as folks
> > > > who have recently been touching `APFloat`.
> > > >
> > > > `APValue` is a Clang-specific class, and it already has some amount of
> > > > serialization support, it seems
> > > > (https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/APValue.h#L54).
> > > > From a quick look, it seems we're already using `APValue` in a
> > > > reasonable number of places in the interpreter, so it might make sense
> > > > to use this object consistently to represent all values in the new
> > > > interpreter?
> > > Going through `APInt` is already possible in `APFloat`, and that might
> > > have some methods to go to char arrays.
> > Yeah - IR gets serialized APFloats somehow (maybe through some layers of
> > indirection) so I'd suggest looking at how, say, a global float named
> > constant gets lowered to bitcode to see how that APFloat is serialized.
> >
> > If that's not enough of a pointer for you/others to find something helpful,
> > I can look further to see what I can find (I got some of the way - a small
> > example with just `extern const float v; const float v = 3.14;` and
> > breaking on APFloat's ctor immediately finds the APFLoat being constructed
> > from that literal - then I looked at llvm::GlobalVariable's ctor, which
> > does break on the creation of `v`, but didn't continue further to figure
> > out how the parsed value gets into the IR constant)
> So, I think this is implemented in `emitGlobalConstantFP()`:
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp#L3206.
> I now wonder how to implement this, since the number of chunks isn't fixed.
> And all this seems rather costly given that we'd have to do this every time
> we access a floating point value on the stack.
That looks like the lowering to assembly - I meant the lowering to LLVM IR
bitcode, though perhaps it's similarly complicated.
As for variable length - I expect the strings encoding had variable length too,
but either relied on null termination or a length prefix. The APFloat
serialization could use a length prefix encoding to specify how many bytes to
read/decode (I guess LLVM IR bitcode does something like that - though I think
the bitcode records always encode their length, so it might be implicit).
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
