Bug 130756

Summary:

[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

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, ggaren, mark.lam, mhahnenberg, msaboff, oliver, sam

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

133034, 133042, 133105

Bug Blocks:

129588, 133052

Attachments:

Description	Flags
it begins	none
work in progress	none
done?	none
more...	none
omg it compiles	none
"getting" there	none
it's back to compiling	none
it begins (setter inlining)	none
more!	none
the code has been written	none
it compiles	none
the patch	oliver: review+

Filip Pizlo

Reported 2014-03-25 16:31:00 PDT

...

Attachments
it begins (14.55 KB, patch) 2014-05-17 22:57 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (31.36 KB, patch) 2014-05-18 12:12 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
done? (37.81 KB, patch) 2014-05-18 12:43 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
more... (51.42 KB, patch) 2014-05-18 18:57 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
omg it compiles (53.88 KB, patch) 2014-05-18 19:23 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
"getting" there (72.83 KB, patch) 2014-05-19 12:58 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it's back to compiling (75.59 KB, patch) 2014-05-19 19:13 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it begins (setter inlining) (10.30 KB, patch) 2014-06-28 17:44 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
more! (33.43 KB, patch) 2014-06-29 20:46 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the code has been written (47.84 KB, patch) 2014-06-30 15:31 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it compiles (57.07 KB, patch) 2014-06-30 19:16 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (70.01 KB, patch) 2014-07-01 13:14 PDT, Filip Pizlo	oliver: review+	Details Formatted Diff Diff
Show Obsolete (11) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2014-05-17 22:57:42 PDT

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

Comment 2 2014-05-18 12:12:20 PDT

Created attachment 231661 [details] work in progress

Filip Pizlo

Comment 3 2014-05-18 12:43:02 PDT

Created attachment 231662 [details] done?

Filip Pizlo

Comment 4 2014-05-18 18:57:47 PDT

Created attachment 231671 [details] more...

Filip Pizlo

Comment 5 2014-05-18 19:23:19 PDT

Created attachment 231672 [details] omg it compiles

Filip Pizlo

Comment 6 2014-05-19 12:58:20 PDT

Created attachment 231713 [details] "getting" there

Filip Pizlo

Comment 7 2014-05-19 19:13:44 PDT

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

Filip Pizlo

Comment 8 2014-05-19 20:53:58 PDT

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

Filip Pizlo

Comment 9 2014-05-20 18:11:43 PDT

What is left is to extend this functionality to setters.

Filip Pizlo

Comment 10 2014-06-28 17:44:36 PDT

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

Filip Pizlo

Comment 11 2014-06-29 20:46:43 PDT

Created attachment 234065 [details] more!

Filip Pizlo

Comment 12 2014-06-30 15:31:28 PDT

Created attachment 234105 [details] the code has been written

Filip Pizlo

Comment 13 2014-06-30 19:16:03 PDT

Created attachment 234139 [details] it compiles

Filip Pizlo

Comment 14 2014-07-01 13:14:26 PDT

Created attachment 234193 [details] the patch

Oliver Hunt

Comment 15 2014-07-01 14:41:38 PDT

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

Comment 16 2014-07-01 14:44:38 PDT

(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

Comment 17 2014-07-01 15:40:51 PDT

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

Note You need to log in before you can comment on or make changes to this bug.