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

[John McCall via Phabricator via llvm-commits]

rjmccall added inline comments.


================
Comment at: llvm/include/llvm/ADT/APFixedPoint.h:67
 
+  bool canAccommodateFloatSemantics(const fltSemantics &FloatSema) const;
+
----------------
ebevhan wrote:
> rjmccall wrote:
> > This should have a doc comment, which should clarify that precision loss is acceptable as long as it doesn't overflow.  Also,  "accommodate" seems like the wrong direction for this: I'd expect that a fixed-point type can "accommodate" a floating-point type if the floating-point values are representable as the fixed-point type, not the reverse.  Maybe `fitsInFloatSemantics`?
> Missed the doccomment by mistake.
> 
> I changed it to 'fitsInFloatSemantics'. It might be a bit misleading though, since it doesn't really check if the real value fits, but rather the value as an integer. That's why I went with something a bit more vague like "accommodate".
Do you just want a method on FixedPointSemantics that returns the unscaled, i.e. "value as an integer" semantics?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85961