[webkit-reviews] review denied: [Bug 85752] Truncating multiplication on integers should not OSR exit every time : [Attachment 140441] the patch

[bugzilla-daemon at webkit.org]

Gavin Barraclough <barraclough at apple.com> has denied Filip Pizlo
<fpizlo at apple.com>'s request for review:
Bug 85752: Truncating multiplication on integers should not OSR exit every time
https://bugs.webkit.org/show_bug.cgi?id=85752

Attachment 140441: the patch
https://bugs.webkit.org/attachment.cgi?id=140441&action=review

------- Additional Comments from Gavin Barraclough <barraclough at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=140441&action=review


r-, because I really do think we need a comment here – but nice optimization,
looks great otherwise.

> Source/JavaScriptCore/dfg/DFGGraph.h:512
> +	   int32_t twoToThe17 = 131072;

Why 2^17?

A number with an absolute value less than 2^17 is representable by 17 bits. 
The result of multiplying a value representable by 17 bits by 2^31 (the largest
possible absolute value of a signed integer) is representable in a 48 bit
mantissa (with additional representation for the sign).  Doubles support 53
bits of mantissa precision.  Put the other way around, for the result of a
multiply to be constrained within a 53 bits mantissa, if I know that the
absolute value of one operand is no more than 2^31 then the second operand may
have a magnitude of +/- 2^22 (less than/greater than – exclusive!).

I think 2^22 is correct here, but either way this really needs a comment. :-)

Also, I'd suggest this would slightly nicer written ''int32_t  twoToThe17 = 1
<< 17;".