[webkit-reviews] review granted: [Bug 109900] DFG AbstractState should filter operands to NewArray more precisely : [Attachment 188489] the patch

[bugzilla-daemon at webkit.org]

Mark Hahnenberg <mhahnenberg at apple.com> has granted Filip Pizlo
<fpizlo at apple.com>'s request for review:
Bug 109900: DFG AbstractState should filter operands to NewArray more precisely
https://bugs.webkit.org/show_bug.cgi?id=109900

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

------- Additional Comments from Mark Hahnenberg <mhahnenberg at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=188489&action=review


r=me

>>> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:1300
>>> +		     forNode(m_graph.m_varArgChildren[node->firstChild() +
operandIndex]).filter(SpecRealNumber);
>> 
>> Why not SpecDouble? What would happen if somebody filled their arrays with
NaNs?
> 
> NaNs can't be stored into double arrays.  If you do it, they turn into
contiguous arrays (of generic JSValues).  The backend will speculate that
you're not storing NaN into a double array and spec fail if you do (so that the
baseline JIT can do the double->contiguous conversion).
> 
> Hence, filtering SpecRealNumber accurately represents the speculations that
the backend will do.

Sounds good.