[PATCH] D85961: [Fixed Point] Add floating point methods to APFixedPoint.

[Bevin Hansson via Phabricator via llvm-commits]

ebevhan added a comment.

I hadn't considered half precision as that's sort of off my radar. That does make both these conversion methods and the corresponding codegen implementation rather problematic.

Would it be completely unthinkable to "promote" calculations to a larger FP type (both here and in codegen) if the exponent bits are insufficient to hold the necessary scaling?



================
Comment at: llvm/lib/Support/APFixedPoint.cpp:431
+  APFloat Flt(FloatSema);
+  APFloat::opStatus S = Flt.convertFromAPInt(Val, Sema.isSigned(), RM);
+
----------------
rjmccall wrote:
> This can overflow the format and result in infinity.  Maybe APFloat just needs a method to do this given an APInt and a binary exponent?  It should be as simple as putting the bits in the right place and then calling normalize().
You're right. I originally only considered this to be a problem when the overflow resulted in a value that wouldn't be representable, but it's clearly not the case. 0.9999999999ur to _Float16 produces infinity. For all common fixed-point scales, single precision floating point and higher should be fine, though.

I'm surprised there isn't already a method to construct an APFloat from its constituent components. It's probably because there are different, incompatible formats so it is not safe to assume that an APFloat is constructed in a particular way.


================
Comment at: llvm/lib/Support/APFixedPoint.cpp:473
+  // It is fine if this overflows to infinity even for saturating types,
+  // since we will use floating point comparisons to check for saturation.
+  APFloat ScaleFactor(std::pow(2, DstFXSema.getScale()));
----------------
rjmccall wrote:
> I don't understand.  It must be possible to have a value that's representable in both the source floating-point type and the destination fixed-point type but not after shifting.
> 
> Maybe you can add methods on APFloat that just extract the denormalized significand and exponent?
There is frexp, but it doesn't return the mantissa as an APInt. I suppose it is possible to bitcast the mantissa out of there.

I wonder how well that works on the non-IEEE format, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85961