Bug 184400 – [JSC] Introduce @putByIdDirectPrivate

• Component: New Bugs
• Status: RESOLVED FIXED

[Yusuke Suzuki]

[JSC] Introduce @putByIdDirectPrivate

[Yusuke Suzuki]

Created attachment 337460 [details]
Patch

[Saam Barati]

Comment on attachment 337460 [details]
Patch

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

r=me

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:994
> +RegisterID* BytecodeIntrinsicNode::emit_intrinsic_putByIdDirect(BytecodeGenerator& generator, RegisterID* dst)

Do we need this since it's unused?

[Saam Barati]

Comment on attachment 337460 [details]
Patch

Would also be good to add a test that shows how this fixes the property being in the prototype chain.

[Caio Lima]

I think it would be good explain in the ChangeLog the reason for introduction of this new intrinsic. It is not straight forward figure out that just looking into the code.

[Yusuke Suzuki]

Comment on attachment 337460 [details]
Patch

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

>> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:994
>> +RegisterID* BytecodeIntrinsicNode::emit_intrinsic_putByIdDirect(BytecodeGenerator& generator, RegisterID* dst)
> 
> Do we need this since it's unused?

I'll keep this to test this feature easily.
I added the test for this.

[Yusuke Suzuki]

(In reply to Caio Lima from comment #4)
> I think it would be good explain in the ChangeLog the reason for
> introduction of this new intrinsic. It is not straight forward figure out
> that just looking into the code.

Added

[Yusuke Suzuki]

Committed r230459: <https://trac.webkit.org/changeset/230459>

[Radar WebKit Bug Importer]

<rdar://problem/39300783>

[Saam Barati]

This or https://bugs.webkit.org/show_bug.cgi?id=184372 seems to have caused a 3% Kraken regression.

[Saam Barati]

(In reply to Saam Barati from comment #9)
> This or https://bugs.webkit.org/show_bug.cgi?id=184372 seems to have caused
> a 3% Kraken regression.

Ignore me, surprisingly, it seems this is responsible: https://bugs.webkit.org/show_bug.cgi?id=184322

[Saam Barati]

Comment on attachment 337460 [details]
Patch

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

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:1022
> +    return generator.moveToDestinationIfNeeded(dst, generator.emitDirectPutById(base.get(), *ident, value.get(), PropertyNode::KnownDirect));

I just thought of this: We should probably make emitDirectPutById talk to the static property analyzer since we use this inside constructors

[Yusuke Suzuki]

(In reply to Saam Barati from comment #11)
> Comment on attachment 337460 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=337460&action=review
> 
> > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:1022
> > +    return generator.moveToDestinationIfNeeded(dst, generator.emitDirectPutById(base.get(), *ident, value.get(), PropertyNode::KnownDirect));
> 
> I just thought of this: We should probably make emitDirectPutById talk to
> the static property analyzer since we use this inside constructors

emitDirectPutById contacts with StaticPropertyAnalyzer :)