[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()

[Steve Canon via Phabricator via cfe-commits]

scanon added inline comments.


================
Comment at: lib/builtins/floattitf.c:65
+        if (a & ((tu_int)1 << LDBL_MANT_DIG)) {
+            a >>= 1;
+            ++e;
----------------
mgorny wrote:
> scanon wrote:
> > Strictly speaking there's no need to adjust `a` here. If we rounded up into a new binade, then `a` is necessarily `0b1000...0`, and the leading 1 bit will get killed by the mask when we assemble `fb.u.high.all` regardless of its position. Same comment applies to floatuntitf.
> I'm sorry but I don't feel confident changing that. AFAIU if the LDBL_MANT_DIG+1 bit is set, this code shifts it lower, so it won't actually be killed by the mask.
In binary128, as in all IEEE 754 binary interchange format encodings, the leading bit of the significand is implicit. The only way to end up in this code path is `0b111...1` rounding up to `0b100...00`, meaning that the significand is 1.0, which is stored as all-zeros (i.e. the leading bit is necessarily masked).

To be more explicit, LDBL_MANT_DIG is 113. If this shift happens, after the shift bit 112 is set, and bits 111:0 are zero. The mask `((a >> 64) & 0x0000ffffffffffffLL)` discards bit 112 (= 64 + 48).


https://reviews.llvm.org/D27898