[PATCH] Fix silent overflow bug in APFloat::multiplySignificand (PR20728).

[Stephen Canon]

This looks reasonable to me.

– Steve

> On Nov 18, 2014, at 8:13 PM, Lang Hames <lhames at gmail.com> wrote:
> 
> Hi All,
> 
> I've been digging in to http://llvm.org/PR20728 <http://llvm.org/PR20728> and I've discovered the root cause of the bug detailed there. The short version is that on x86 the following call should constant-fold to 4.0L:
> 
> fmal(1.0L, 1.0L, 3.0L) 
> 
> However, it currently constant folds to 0.0L.
> 
> Here's what goes wrong: The call is constant folded using APFloat::fusedMultiplyAdd, but the real work is done by APFloat::multiplySignificand. APFloat::multiplySignificand takes an RHS for the multiplication and an optional addend. The method allocates a 2x precision intermediate significand to hold the result of both operations. This is sufficient to hold the result of the multiplication without overflowing, but if the MSB of the multiplication result and the addend are both in the top bits of their respective significands, the addition operation will overflow. For reasons too perverse to detail here (read the bug for the gory details) this overflow is not detected, and multiplySignificand happily returns bogus results. In the case of the test case above, you end up with:
> 
>     0100...0  (1.0)
> +   1100...0  (3.0)
> ---------------------
> 1 | 0000...0  (0.0!)
> 
> The attached patch fixes this bug by holding the intermediate result in 2 * precision + 1 bits, and arranging for the top bit of the addend to be zero (by shifting right by 1). This means that the addition no longer overflows, but simply carries into the top bit.
> 
> This patch passes the regression test suite and the nightly test suite on x86-64, so that's a good start. Are there any APFloat experts out there who are able to offer comment on the approach I've taken though? It seems superficially sane, but this is floating point code - the appearance of sanity may be purely coincidental.
> 
> Cheers,
> Lang.
> <APFloat_multiplySignificand_fix.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141118/82d9e054/attachment.html>