[webkit-reviews] review granted: [Bug 131423] DFG IR should keep the data flow of doubles and int52's separate from the data flow of JSValue's : [Attachment 229357] the patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 131423: DFG IR should keep the data flow of doubles and int52's separate
from the data flow of JSValue's
https://bugs.webkit.org/show_bug.cgi?id=131423

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

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=229357&action=review


r=me

Please make sure to test 32bit and iOS before landing.

> Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:263
> +		   // DoubleAsInt42 node, which occurs after the Div/Mod node
that the conversions

DoubleAsInt52?

> Source/JavaScriptCore/dfg/DFGNodeType.h:119
> +    macro(ValueRep, NodeResultJS) \

Seems like we should rename NodeResultJS to NodeResultValue to match.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2168
> +	   // This is pretty annoying - boxDouble() on X86 clobbers the source.
That's kinda gross.

A better way to say this is, here, "boxDouble() on X86 clobbers the source, so
we need to make a copy", and then in the assembler, "FIXME: Don't clobber the
source."