Bug 59405

Summary: DFG JIT - add type speculation for integer & array types, for vars & args.
Product: WebKit Reporter: Gavin Barraclough <barraclough>
Component: JavaScriptCoreAssignee: Gavin Barraclough <barraclough>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
The patch ggaren: review+

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.