mgorny added inline comments.

================
Comment at: lib/builtins/floattitf.c:65
+        if (a & ((tu_int)1 << LDBL_MANT_DIG)) {
+            a >>= 1;
+            ++e;
----------------
scanon wrote:
> 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).
Well, I've tried removing this and it causes one of the tests to fail:

`error in __floatuntitf(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF) = 0X1P+127, 
expected 0X1P+128`


https://reviews.llvm.org/D27898



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to