Bug 232168 – [JSC] GetTypedArrayLengthAsInt52 must be inserted only when we ensure that input is TypedArray via array-mode-based filtering

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

[Yusuke Suzuki]

[JSC] GetTypedArrayLengthAsInt52 must be inserted only when we ensure that input is TypedArray via array-mode-based filtering

[Yusuke Suzuki]

Created attachment 442188 [details]
Patch

[Yusuke Suzuki]

<rdar://problem/84366658>

[Yusuke Suzuki]

Created attachment 442192 [details]
Patch

[Robin Morisset]

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.

[Yusuke Suzuki]

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.

[Robin Morisset]

Comment on attachment 442192 [details]
Patch

r=me

[Yusuke Suzuki]

Committed r284716 (243431@main): <https://commits.webkit.org/243431@main>