Bug 232168 - [JSC] GetTypedArrayLengthAsInt52 must be inserted only when we ensure that input is TypedArray via array-mode-based filtering
Summary: [JSC] GetTypedArrayLengthAsInt52 must be inserted only when we ensure that in...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-10-22 13:10 PDT by Yusuke Suzuki
Modified: 2021-10-22 14:56 PDT (History)
8 users (show)

See Also:

Attachments
Patch (3.46 KB, patch)
2021-10-22 13:12 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (4.07 KB, patch)
2021-10-22 13:25 PDT, Yusuke Suzuki 		rmorisset: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2021-10-22 13:10:39 PDT 
[JSC] GetTypedArrayLengthAsInt52 must be inserted only when we ensure that input is TypedArray via array-mode-based filtering
Comment 1 Yusuke Suzuki 2021-10-22 13:12:26 PDT 
Created attachment 442188 [details]
Patch
Comment 2 Yusuke Suzuki 2021-10-22 13:12:29 PDT 
<rdar://problem/84366658>
Comment 3 Yusuke Suzuki 2021-10-22 13:25:29 PDT 
Created attachment 442192 [details]
Patch
Comment 4 Robin Morisset 2021-10-22 13:33:11 PDT 
Comment on attachment 442192 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp:102
> +            if (m_node->arrayMode().isSomeTypedArrayView() && m_node->arrayMode().isOutOfBounds()) {

This actually increases how often we take that code path, as isSomeTypedArrayView() returns true for AnyTypedArray whose typedArrayTyped() is NotTypedArray.
Is it on purpose?

> Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp:152
> +        if ((op == GetArrayLength) && m_node->arrayMode().isSomeTypedArrayView() && (m_node->arrayMode().mayBeLargeTypedArray() || m_graph.hasExitSite(m_node->origin.semantic, Overflow))) {

Ah, this is the bug, thanks for finding it.
Comment 5 Yusuke Suzuki 2021-10-22 13:38:00 PDT 
Comment on attachment 442192 [details]
Patch

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

>> Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp:102
>> +            if (m_node->arrayMode().isSomeTypedArrayView() && m_node->arrayMode().isOutOfBounds()) {
> 
> This actually increases how often we take that code path, as isSomeTypedArrayView() returns true for AnyTypedArray whose typedArrayTyped() is NotTypedArray.
> Is it on purpose?

Yes. Currently both will not get AnyArrayType value since it will be used only for some intrinsics, and they are not emitting GetByVal, PutByVal etc.
But I aligned the implementation to using isSomeTypedArrayView since permitsBoundsCheckLowering (this is in the prologue of lowerBoundsCheck) is accepting AnyArrayType, and getting typed array length should work for AnyArrayType type too.

>> Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp:152
>> +        if ((op == GetArrayLength) && m_node->arrayMode().isSomeTypedArrayView() && (m_node->arrayMode().mayBeLargeTypedArray() || m_graph.hasExitSite(m_node->origin.semantic, Overflow))) {
> 
> Ah, this is the bug, thanks for finding it.

Ditto.
Comment 6 Robin Morisset 2021-10-22 13:39:11 PDT 
Comment on attachment 442192 [details]
Patch

r=me
Comment 7 Yusuke Suzuki 2021-10-22 14:56:10 PDT 
Committed r284716 (243431@main): <https://commits.webkit.org/243431@main>