WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
182792
[FTL] Support HasIndexedProperty for ArrayStorage and SlowPutArrayStorage
https://bugs.webkit.org/show_bug.cgi?id=182792
Summary
[FTL] Support HasIndexedProperty for ArrayStorage and SlowPutArrayStorage
Yusuke Suzuki
Reported
2018-02-14 06:16:36 PST
[FTL] Support HasIndexedProperty for ArrayStorage
Attachments
Patch
(6.70 KB, patch)
2018-02-14 06:17 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(10.72 KB, patch)
2018-02-19 23:16 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1
2018-02-14 06:17:38 PST
Created
attachment 333791
[details]
Patch
Saam Barati
Comment 2
2018-02-19 19:02:56 PST
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 3
2018-02-19 23:06:20 PST
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
Comment 4
2018-02-19 23:16:55 PST
Created
attachment 334244
[details]
Patch
Mark Lam
Comment 5
2018-02-22 20:47:58 PST
Comment on
attachment 334244
[details]
Patch r=me
Yusuke Suzuki
Comment 6
2018-02-22 22:18:23 PST
Comment on
attachment 334244
[details]
Patch Thanks!
WebKit Commit Bot
Comment 7
2018-02-22 22:41:54 PST
Comment on
attachment 334244
[details]
Patch Clearing flags on attachment: 334244 Committed
r228943
: <
https://trac.webkit.org/changeset/228943
>
WebKit Commit Bot
Comment 8
2018-02-22 22:41:55 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9
2018-02-22 22:42:25 PST
<
rdar://problem/37816653
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug