Bug 130756 – [ftlopt] DFG bytecode parser should turn PutById with nothing but a Setter stub as stuff+handleCall, and handleCall should be allowed to inline if it wants to

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

...

[Filip Pizlo]

Created attachment 231649 [details]
it begins

Saving my work while I work on one of the required bits in a separate bug: https://bugs.webkit.org/show_bug.cgi?id=133042

[Filip Pizlo]

Created attachment 231661 [details]
work in progress

[Filip Pizlo]

Created attachment 231662 [details]
done?

[Filip Pizlo]

Created attachment 231671 [details]
more...

[Filip Pizlo]

Created attachment 231672 [details]
omg it compiles

[Filip Pizlo]

Created attachment 231713 [details]
"getting" there

[Filip Pizlo]

Created attachment 231748 [details]
it's back to compiling

[Filip Pizlo]

I'll go ahead and do just the getter part separately, in https://bugs.webkit.org/show_bug.cgi?id=133105

[Filip Pizlo]

What is left is to extend this functionality to setters.

[Filip Pizlo]

Created attachment 234056 [details]
it begins (setter inlining)

[Filip Pizlo]

Created attachment 234065 [details]
more!

[Filip Pizlo]

Created attachment 234105 [details]
the code has been written

[Filip Pizlo]

Created attachment 234139 [details]
it compiles

[Filip Pizlo]

Created attachment 234193 [details]
the patch

[Oliver Hunt]

Comment on attachment 234193 [details]
the patch

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

> Source/JavaScriptCore/tests/stress/weird-setter-counter.js:20
> +    var o = {};
> +    o.__defineSetter__("f", function(value) {

While it logically shouldn't matter could you also add tests using {set f(value) { ... }} just to make sure we're not doing anything substantially different?

[Filip Pizlo]

(In reply to comment #15)
> (From update of attachment 234193 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=234193&action=review
> 
> > Source/JavaScriptCore/tests/stress/weird-setter-counter.js:20
> > +    var o = {};
> > +    o.__defineSetter__("f", function(value) {
> 
> While it logically shouldn't matter could you also add tests using {set f(value) { ... }} just to make sure we're not doing anything substantially different?

That makes sense.  How about I just replicate weird-setter-counter in this way, since if I replicate all of the tests in this way then we'll have too much overlapping coverage.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/170672