[webkit-reviews] review granted: [Bug 172421] [DFG] Add ArrayIndexOf intrinsic : [Attachment 312485] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 172421: [DFG] Add ArrayIndexOf intrinsic
https://bugs.webkit.org/show_bug.cgi?id=172421

Attachment 312485: Patch

https://bugs.webkit.org/attachment.cgi?id=312485&action=review




--- Comment #19 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 312485
  --> https://bugs.webkit.org/attachment.cgi?id=312485
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312485&action=review

r=me with a few comments.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7516
> +    case DoubleRepUse: {

Nit: Worth an assertion that the array mode is double here.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7556
> +    case ObjectUse: {

Style nit: This is almost identical to the int32 loop above (besides the 32-bit
tag code). Maybe you can abstract it?

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4097
> +	       LValue index = m_out.phi(Int32, initialStartIndex);

Nit: It might be worth doing 64 bit math and having the zeroExtend outside the
loop. (Only if we actually end up emitting machine instructions to zero extend
inside the loop)

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4098
> +	       ValueFromBlock notFoundResult =
m_out.anchor(m_out.constInt32(-1));

What code do we generate here? Do we end up emitting a Mov, -1, Reg,  inside
the loop every time? If so, it might be faster to branch to a block that sets
not found result.