Skip to content

Avoid right-shifting a negative value in detail::power() - #426

Open
sahvx655-wq wants to merge 1 commit into
fastfloat:mainfrom
sahvx655-wq:power-negative-shift
Open

sahvx655-wq wants to merge 1 commit into
fastfloat:mainfrom
sahvx655-wq:power-negative-shift

Conversation

@sahvx655-wq

Copy link
Copy Markdown
Contributor

detail::power computes (((152170 + 65536) * q) >> 16) + 63, and q is negative for every conversion with a negative decimal exponent ("0.25", "1e-300"), so the shift acts on a negative value, which is implementation-defined before C++20. The derivation in the surrounding comment needs the shift to floor toward negative infinity, which only an arithmetic shift gives. 82623d8 removed the same construct from subnormal_ties_possible(); this sibling site is the last negative right shift in the headers.

Bias the product by 2048 * 65536 before the shift and subtract 2048 afterwards. The bias is a multiple of 2^16, so the result is unchanged for every q in the documented (-400, 350) range; I checked all of them against a well-defined floor reference, and the biased product stays within int range. The full test suite passes, with the constexpr tests and the sanitizer build, and the canada.txt benchmark is flat on an Apple M-class machine (2275 to 2287 MB/s before, 2276 to 2289 MB/s after): the one added constant sits off the critical mul/shift dependency chain.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant