Bug 59405 – DFG JIT - add type speculation for integer & array types, for vars & args.

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

[Gavin Barraclough]

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.

[Gavin Barraclough]

Created attachment 91030 [details]
The patch

[WebKit Review Bot]

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.

[Geoffrey Garen]

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.

[Gavin Barraclough]

Fixed in r84860.