Bug 129210 – FTL should do polymorphic PutById inlining

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

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 224977 [details]
it begins

[Filip Pizlo]

Created attachment 224980 [details]
profiling side is done

[Filip Pizlo]

Created attachment 224990 [details]
more stuff

[Filip Pizlo]

Created attachment 224991 [details]
almost done

[Filip Pizlo]

Created attachment 225023 [details]
the code is written

[Filip Pizlo]

Created attachment 225025 [details]
the patch

[WebKit Commit Bot]

Attachment 225025 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/PutByIdStatus.cpp:161:  Wrong number of spaces before statement. (expected: 24)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:187:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2011:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2024:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2028:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2043:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2045:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2052:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:398:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:407:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:409:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:421:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:423:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:434:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:436:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:456:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:461:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:465:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 18 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

(In reply to comment #7)
> Attachment 225025 [details] did not pass style-queue:
> 
> 
> ERROR: Source/JavaScriptCore/bytecode/PutByIdStatus.cpp:161:  Wrong number of spaces before statement. (expected: 24)  [whitespace/indent] [4]

You expected wrong.

> ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:187:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]

Fixed.

> ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2011:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2024:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2028:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2043:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2045:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2052:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:398:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:407:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:409:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:421:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:423:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:434:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:436:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:456:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:461:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
> ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:465:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]

Fixed.

> Total errors found: 18 in 32 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 225026 [details]
the patch

[WebKit Commit Bot]

Attachment 225026 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/PutByIdStatus.cpp:161:  Wrong number of spaces before statement. (expected: 24)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2011:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 2 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 225071 [details]
the patch

[WebKit Commit Bot]

Attachment 225071 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/PutByIdStatus.cpp:161:  Wrong number of spaces before statement. (expected: 24)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2011:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 2 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 225073 [details]
the patch

Forgot LayoutTests.

[WebKit Commit Bot]

Attachment 225073 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/PutByIdStatus.cpp:161:  Wrong number of spaces before statement. (expected: 24)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2011:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 2 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Oliver Hunt]

Comment on attachment 225073 [details]
the patch

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

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1969
> +        if (!isFTL(m_graph.m_plan.mode)) {
> +            emitPutById(base, identifierNumber, value, isDirect);
> +            return;
> +        }

We will need to support polymorphic put_by_id in the DFG (and baseline, and possibly even the interpreter) to make caching of DOM setters actually useful.

[Filip Pizlo]

(In reply to comment #15)
> (From update of attachment 225073 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=225073&action=review
> 
> > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1969
> > +        if (!isFTL(m_graph.m_plan.mode)) {
> > +            emitPutById(base, identifierNumber, value, isDirect);
> > +            return;
> > +        }
> 
> We will need to support polymorphic put_by_id in the DFG (and baseline, and possibly even the interpreter) to make caching of DOM setters actually useful.

As I've told you before, we already support caching polymorphic put_by_id in baseline, DFG, and FTL. This patch doesn't change that. This patch has to do with inlining, not caching. 

So if you think that Dom setters are blocked on any of this, then you're in luck because there isn't anything to block on.

[Mark Hahnenberg]

Comment on attachment 225073 [details]
the patch

r=me too!

[Filip Pizlo]

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