Bug 101438 – DFG should not assume that something is a double just because it might be undefined

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

[Filip Pizlo]

Currently our arithmetic paths in the DFG have two, or three, versions:

1) Integer path.  This only accepts integers.

2) Double path.  This only accepts doubles or integers.

3) For ValueAdd only, there is a generic path.

For operations that only have (1) or (2), we currently assume that we must use (2) if either operand may not be a number.  This is clearly pointless, as it means that we do an upconversion to double even when we have no evidence that the value is in fact a double.

For ValueAdd, it probably makes no sense to fall off into the generic path just because the value can be undefined.  Undefined data flow arcs may exist if a variable is conditionally assigned, like in:

var x;
if (p)
   x = 42;
... // stuff
if (p)
   y = x + 1; // Currently we'll use generic ValueAdd here.

We should change the speculation logic to account for the fact that (a) undefined flowing into arithmetic means we've probably already lost anyway and (b) speculating double for something that is either integer or not even a number buys nothing.

[Filip Pizlo]

Created attachment 172738 [details]
the patch

[WebKit Review Bot]

Attachment 172738 [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/DFGPredictionPropagationPhase.cpp:319:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:360:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:384:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:822:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2370:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:2407:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:266:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:284:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 8 in 11 files


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

[Filip Pizlo]

Created attachment 172740 [details]
the patch

Fix style

[Filip Pizlo]

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