Bug 105900 – Baseline JIT should have closure call caching

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

[Filip Pizlo]

This is not meant to be an optimization by itself, but rather, it will enable the DFG inliner to see when a call was a closure call due to the profiling that baseline closure call caching will implicitly give us.

[Filip Pizlo]

Created attachment 180997 [details]
the patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/jit/JIT.cpp:874:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1319:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 12 files


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

[Build Bot]

Comment on attachment 180997 [details]
the patch

Attachment 180997 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15638244

New failing tests:
inspector/debugger/script-formatter.html
jquery/css.html
fast/sub-pixel/sub-pixel-accumulates-to-layers.html
fast/sub-pixel/sub-pixel-iframe-copy-on-scroll.html
fast/sub-pixel/transformed-iframe-copy-on-scroll.html

[Build Bot]

Comment on attachment 180997 [details]
the patch

Attachment 180997 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15625357

New failing tests:
inspector/debugger/script-formatter.html
jquery/css.html
fast/sub-pixel/sub-pixel-accumulates-to-layers.html
fast/sub-pixel/sub-pixel-iframe-copy-on-scroll.html
fast/sub-pixel/transformed-iframe-copy-on-scroll.html

[Gavin Barraclough]

Comment on attachment 180997 [details]
the patch

Seems to be a fair bit of duplication between the 32_64 & 64 privateCompileClosureCall implementations, might be worth considering merging.
r+ assuming that the test failures aren't due to this.

[Filip Pizlo]

(In reply to comment #5)
> (From update of attachment 180997 [details])
> Seems to be a fair bit of duplication between the 32_64 & 64 privateCompileClosureCall implementations, might be worth considering merging.
> r+ assuming that the test failures aren't due to this.

Thanks!

It appears that this was a real regression, which I've fixed by having cti_vm_lazyLinkClosureCall use generatedJITCodeWithArityCheck rather than generatedJITCode if there is an arity mismatch.  This is a one-line change from the patch you reviewed.

[Filip Pizlo]

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

[Csaba Osztrogonác]

(In reply to comment #7)
> Landed in http://trac.webkit.org/changeset/138609

Buildfixes landed in https://trac.webkit.org/changeset/138610 and  https://trac.webkit.org/changeset/138612