Bug 182792 – [FTL] Support HasIndexedProperty for ArrayStorage and SlowPutArrayStorage

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[FTL] Support HasIndexedProperty for ArrayStorage

[Yusuke Suzuki]

Created attachment 333791 [details]
Patch

[Saam Barati]

Comment on attachment 333791 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:3
> +        [FTL] Support HasIndexedProperty for ArrayStorage

And SlowPutArrayStorage.

> Source/JavaScriptCore/ChangeLog:8
> +        This patch adds HasIndexedProperty for ArrayStorage in FTL.

And SlowPutArrayStorage.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9781
> +                m_out.jump(checkHole);

This needs to do a speculation check that it's not out of bounds. Seems like this should be testable via crashing on some OOB read. You also won't need a "checkHole" dedicated block if it's in bounds, you can just use the incoming block we're emitting code in.

[Yusuke Suzuki]

Comment on attachment 333791 [details]
Patch

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

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9781
>> +                m_out.jump(checkHole);
> 
> This needs to do a speculation check that it's not out of bounds. Seems like this should be testable via crashing on some OOB read. You also won't need a "checkHole" dedicated block if it's in bounds, you can just use the incoming block we're emitting code in.

As is the same to GetByVal etc., HasIndexedProperty also has special lowering rule in SSA lowering phase, which emits CheckInBounds.
We do not need to perform out-of-bounds speculation here since preceeding CheckInBounds does this. For ArrayStorage/SlowPutArrayStorage, we emit GetVectorLength and CheckInBounds instead of GetArrayLength and CheckInBounds. So it is correctly handled.

For checkHole case, I'll update the patch to drop checkHole BB for InBounds case.

[Yusuke Suzuki]

Created attachment 334244 [details]
Patch

[Mark Lam]

Comment on attachment 334244 [details]
Patch

r=me

[Yusuke Suzuki]

Comment on attachment 334244 [details]
Patch

Thanks!

[WebKit Commit Bot]

Comment on attachment 334244 [details]
Patch

Clearing flags on attachment: 334244

Committed r228943: <https://trac.webkit.org/changeset/228943>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/37816653>