Bug 105900 - Baseline JIT should have closure call caching
Summary: Baseline JIT should have closure call caching
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on: 105930 111704
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-01 14:20 PST by Filip Pizlo
Modified: 2013-03-07 09:20 PST (History)
9 users (show)

See Also:


Attachments
the patch (22.08 KB, patch)
2013-01-01 14:21 PST, Filip Pizlo
barraclough: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2013-01-01 14:20:22 PST
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.
Comment 1 Filip Pizlo 2013-01-01 14:21:47 PST
Created attachment 180997 [details]
the patch
Comment 2 WebKit Review Bot 2013-01-01 14:24:46 PST
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.
Comment 3 Build Bot 2013-01-01 16:00:40 PST
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
Comment 4 Build Bot 2013-01-01 17:04:48 PST
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
Comment 5 Gavin Barraclough 2013-01-01 17:50:29 PST
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.
Comment 6 Filip Pizlo 2013-01-01 20:31:32 PST
(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.
Comment 7 Filip Pizlo 2013-01-02 00:50:10 PST
Landed in http://trac.webkit.org/changeset/138609
Comment 8 Csaba Osztrogonác 2013-01-02 01:58:07 PST
(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