[PATCH] D69771: [APFloat] Handle exponent underflow correctly

[Ehud Katz via Phabricator via llvm-commits]

ekatz marked 2 inline comments as done.
ekatz added inline comments.


================
Comment at: llvm/include/llvm/ADT/APFloat.h:146
   /// A signed type to represent a floating point numbers unbiased exponent.
-  typedef signed short ExponentType;
+  typedef int32_t ExponentType;
 
----------------
efriedma wrote:
> What's the effect of this on the size of APFloat on a 64-bit target?  On a 32-bit target?
`APFloat` remains the same size. I added a note about that in the description. Maybe it is not clear enough.

`detail::IEEEFloat` is the only record that has a field of `ExponentType` type. It remains the size of 24 bytes and alignment of 8 bytes (in both 32 bit and 64 bit targets).


================
Comment at: llvm/lib/Support/APFloat.cpp:994
+  // Save the addend now, before we change our properties, as it may be equal to
+  // 'this' pointer.
+  if (addend)
----------------
efriedma wrote:
> Does this have some functional effect?  If it does, is there a test?
Now that I think of it, I'll just separate it into another patch. It is a bug, revealed from the tests I've made.


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

https://reviews.llvm.org/D69771