Bug 230086 – [JSC] Optimize op_get_property_enumerator further

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

[Yusuke Suzuki]

[JSC] Optimize op_get_property_enumerator further

[Yusuke Suzuki]

Created attachment 437718 [details]
Patch

[Yusuke Suzuki]

Created attachment 437719 [details]
Patch

[Saam Barati]

Comment on attachment 437719 [details]
Patch

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

r=me with one bug to fix and test again

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:13980
> +        if ((baseValue.m_type && !(baseValue.m_type & ~SpecObject)) && baseValue.m_structure.isFinite()) {

Nit: first two clauses can be baseValue.isType(SpecObject)

We don’t care about checking zero since zero just means we couldn’t be executing at this point anyways.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:13256
> +            if ((baseValue.m_type && !(baseValue.m_type & ~SpecObject)) && baseValue.m_structure.isFinite()) {

Ditto. Can use isType

> Source/JavaScriptCore/jit/JITPropertyAccess.cpp:2816
> +    genericCases.link(this);

Why not use the baseline JIT’s slow path mechanism?

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:3100
> +    loadb JSCell::m_type[t0], t1

Should be m_indexingTypeAndMisc. I wonder if this just made us go slow all the time. Worth checking the rest of the fast path is correct.

[Yusuke Suzuki]

Comment on attachment 437719 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:13980
>> +        if ((baseValue.m_type && !(baseValue.m_type & ~SpecObject)) && baseValue.m_structure.isFinite()) {
> 
> Nit: first two clauses can be baseValue.isType(SpecObject)
> 
> We don’t care about checking zero since zero just means we couldn’t be executing at this point anyways.

Right! Fixed.

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:13256
>> +            if ((baseValue.m_type && !(baseValue.m_type & ~SpecObject)) && baseValue.m_structure.isFinite()) {
> 
> Ditto. Can use isType

Fixed.

>> Source/JavaScriptCore/jit/JITPropertyAccess.cpp:2816
>> +    genericCases.link(this);
> 
> Why not use the baseline JIT’s slow path mechanism?

Because this is still not slow path. It is used if indexed properties exist. We have a super fast path inlined, but this does not mean that this slowpath will not be used.

>> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:3100
>> +    loadb JSCell::m_type[t0], t1
> 
> Should be m_indexingTypeAndMisc. I wonder if this just made us go slow all the time. Worth checking the rest of the fast path is correct.

Oops, fixed.

[Yusuke Suzuki]

Committed r282239 (241523@main): <https://commits.webkit.org/241523@main>

[Radar WebKit Bug Importer]

<rdar://problem/82943743>

[Yusuke Suzuki]

*** Bug 229888 has been marked as a duplicate of this bug. ***

[Yusuke Suzuki]

*** Bug 229889 has been marked as a duplicate of this bug. ***