[PATCH] D29109: [APFloat] Fix comments. NFC.

Tim Shen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 25 16:05:28 PST 2017


timshen added inline comments.


================
Comment at: llvm/include/llvm/ADT/APFloat.h:1065
+  /// Add two APFloats using \c rmNearestTiesToEven rounding mode.
+  /// *No* error checking.
   APFloat operator+(const APFloat &RHS) const {
----------------
jlebar wrote:
> Nit, I'm not sure we need the asterisks around "No".
Removed. I don't like it either.


================
Comment at: llvm/lib/Support/APFloat.cpp:69
 
-  /* The PowerPC format consists of two doubles.  It does not map cleanly
-     onto the usual format above.  It is approximated using twice the
-     mantissa bits.  Note that for exponents near the double minimum,
-     we no longer can represent the full 106 mantissa bits, so those
-     will be treated as denormal numbers.
-
-     FIXME: While this approximation is equivalent to what GCC uses for
-     compile-time arithmetic on PPC double-double numbers, it is not able
-     to represent all possible values held by a PPC double-double number,
-     for example: (long double) 1.0 + (long double) 0x1p-106
-     Should this be replaced by a full emulation of PPC double-double?
+  /* The IBM double-double numbers. It consists of a pair of IEEE 64-bit
+     doubles (Hi, Lo), where |Hi| > |Lo|, and (double)(Hi + Lo) == Hi.
----------------
jlebar wrote:
> s/It consists/They consist/ to match "number*s*" earlier.
Tweaked a bit.


================
Comment at: llvm/lib/Support/APFloat.cpp:70
+  /* The IBM double-double numbers. It consists of a pair of IEEE 64-bit
+     doubles (Hi, Lo), where |Hi| > |Lo|, and (double)(Hi + Lo) == Hi.
+     The numeric value it's modeling is Hi + Lo. Therefore it has two 53-bit
----------------
jlebar wrote:
> Well, that second criterion is only true for non-denormals, right?
Added ", and if normalized, ..."


https://reviews.llvm.org/D29109





More information about the llvm-commits mailing list