[webkit-reviews] review granted: [Bug 230086] [JSC] Optimize op_get_property_enumerator further : [Attachment 437719] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 230086: [JSC] Optimize op_get_property_enumerator further
https://bugs.webkit.org/show_bug.cgi?id=230086

Attachment 437719: Patch

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




--- Comment #3 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 437719
  --> https://bugs.webkit.org/attachment.cgi?id=437719
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.