Page MenuHomePhabricator

APFloat: Fix scalbn handling of denormals
ClosedPublic

Authored by arsenm on Mar 12 2016, 4:00 AM.

Details

Reviewers
scanon
Summary

This was incorrect for denormals, and also failed
on longer exponent ranges.

Diff Detail

Event Timeline

arsenm updated this revision to Diff 50522.Mar 12 2016, 4:00 AM
arsenm retitled this revision from to APFloat: Fix scalbn handling of denormals.
arsenm updated this object.
arsenm added a reviewer: scanon.
arsenm added a subscriber: llvm-commits.
scanon added inline comments.Mar 12 2016, 4:37 AM
lib/Support/APFloat.cpp
3951

This check still isn't right. Most importantly, 'Exp + X.exponent' may overflow if 'Exp' is huge. Much, much less critically, but still worth keeping in mind, on a hypothetical custom format with a significand with more bits than –MinExp the check still isn't right.

Let's let normalize handle all the overflow/underflow so that aspect of control flow is more clear, and to handle non-default rounding correctly, rather than doing it piecemeal. Something like:

// If Exp is wildly out-of-scale, simply adding it to X.exponent will overflow;
// clamp it to a safe range before adding, but ensure that the range is large
// enough that the clamp does not change the result.  The range we need to
// support is the difference between the largest possible exponent and the
// normalized exponent of half the smallest denormal.
int maxIncrement = MaxExp - MinExp + precision;
X.exponent += min(max(Exp,-maxIncrement), maxIncrement);
X.normalize(/* scalbn should take a rounding mode and pass it through here */);
arsenm updated this revision to Diff 50536.Mar 12 2016, 5:59 PM

Clamp Exp to 1 past the maximum increment/decrement. Let normalize handle overflow/underflow

Should this return the status from normalize somewhere?

scanon accepted this revision.Mar 12 2016, 8:46 PM
scanon edited edge metadata.

LGTM, thanks.

This revision is now accepted and ready to land.Mar 12 2016, 8:46 PM
arsenm closed this revision.Mar 12 2016, 9:18 PM

r263369