[webkit-reviews] review denied: [Bug 128155] ASSERT in speculateMachineInt on 32-bit platforms : [Attachment 223064] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has denied  review:
Bug 128155: ASSERT in speculateMachineInt on 32-bit platforms
https://bugs.webkit.org/show_bug.cgi?id=128155

Attachment 223064: Patch
https://bugs.webkit.org/attachment.cgi?id=223064&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=223064&action=review


> Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:139
>	       if (type == SpecInt52AsDouble)
> -		   type = SpecInt52;
> +		   type = enableInt52() ? SpecInt52 : SpecDouble;

I'm pretty sure this is wrong and will result in worse optimizations on 32-bit.
 In particular, you're claiming that if something was representable as an in52
(i.e. it's non-fractional and not-NaN and not infinity), then it's SpecDouble,
which means "this value can be an integer, or a real, or infinity, or NaN,
etc".  So, this is not the right fix.

You should have instead just made this say:

if (type == SpecInt52AsDouble && enableInt52())
    type = SpecInt52;