This was incorrect for denormals, and also failed
on longer exponent ranges.
Details
Details
Diff Detail
Diff Detail
Event Timeline
| 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 */); | |
Comment Actions
Clamp Exp to 1 past the maximum increment/decrement. Let normalize handle overflow/underflow
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: