Bug 59405 - DFG JIT - add type speculation for integer & array types, for vars & args.
Summary: DFG JIT - add type speculation for integer & array types, for vars & args.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-25 17:20 PDT by Gavin Barraclough
Modified: 2011-04-25 18:43 PDT (History)
1 user (show)

See Also:


Attachments
The patch (24.93 KB, patch)
2011-04-25 17:22 PDT, Gavin Barraclough
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2011-04-25 17:20:25 PDT
If a var or argument is used as the base for a GetByVal or PutByVal access we are speculating that it is of type Array (we only generate code on the speculative path to perform array accesses).  By typing the var or args slot as Array, and checking on entry to the function (in the case of args), and each time the local is written to, we can avoid a type check at each point the array is accessed.  This will typically hoist type checks out of loops.

Similarly, any local that is incremented or decremented, or is the input or output or a bitwise operator, is likely to be an integer.  By typing the local as int32 we can avoid speculation checks on access, and tagging when writing to the slot.  All accesses can become 32bit instead of 64.
Comment 1 Gavin Barraclough 2011-04-25 17:22:29 PDT
Created attachment 91030 [details]
The patch
Comment 2 WebKit Review Bot 2011-04-25 17:24:07 PDT
Attachment 91030 [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/dfg/DFGSpeculativeJIT.cpp:931:  Should only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/JavaScriptCore/dfg/DFGNonSpeculativeJIT.cpp:177:  Should only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/JavaScriptCore/dfg/DFGGraph.h:32:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 3 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Geoffrey Garen 2011-04-25 17:30:46 PDT
Comment on attachment 91030 [details]
The patch

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

r=me

> Source/JavaScriptCore/dfg/DFGGraph.h:141
> +        if (operand < 0) {

Probably worth a helper function to abstract away the fact that a negative number means an argument.

> Source/JavaScriptCore/dfg/DFGGraph.h:151
> +        if (operand < 0) {

Ditto.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:290
> +            // integerResult, but don't useChildren!

Not new to this patch, but it would be good to explain the why here (phi nodes), instead of the what.
Comment 4 Gavin Barraclough 2011-04-25 18:43:25 PDT
Fixed in r84860.