Changeset 142544 in webkit for trunk/Source/JavaScriptCore/dfg/DFGVariableAccessData.h

Timestamp:

Feb 11, 2013, 4:29:20 PM (13 years ago)

Author:

[email protected]

Message:

Strange bug in DFG OSR in JSC
​https://bugs.webkit.org/show_bug.cgi?id=109491

Source/JavaScriptCore:

Reviewed by Mark Hahnenberg.

Int32ToDouble was being injected after a side-effecting operation and before a SetLocal. Anytime we
inject something just before a SetLocal we should be aware that the previous operation may have been
a side-effect associated with the current code origin. Hence, we should use a forward exit.
Int32ToDouble does not do forward exits by default.

This patch adds a forward-exiting form of Int32ToDouble, for use in SetLocal Int32ToDouble injections.
Changed the CSE and other things to treat these nodes identically, but for the exit strategy to be
distinct (Int32ToDouble -> backward, ForwardInt32ToDouble -> forward). The use of the NodeType for
signaling exit direction is not "great" but it's what we use in other places already (like
ForwardCheckStructure).

• dfg/DFGAbstractState.cpp:

(JSC::DFG::AbstractState::execute):

• dfg/DFGCSEPhase.cpp:

(JSC::DFG::CSEPhase::int32ToDoubleCSE):
(CSEPhase):
(JSC::DFG::CSEPhase::performNodeCSE):

• dfg/DFGCommon.h:
• dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::fixDoubleEdge):
(JSC::DFG::FixupPhase::injectInt32ToDoubleNode):

• dfg/DFGNode.h:

(JSC::DFG::Node::willHaveCodeGenOrOSR):

• dfg/DFGNodeType.h:

(DFG):

• dfg/DFGPredictionPropagationPhase.cpp:

(JSC::DFG::PredictionPropagationPhase::propagate):

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::convertLastOSRExitToForward):
(JSC::DFG::SpeculativeJIT::compileInt32ToDouble):

• dfg/DFGSpeculativeJIT.h:
• dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• dfg/DFGVariableEventStream.cpp:

(JSC::DFG::VariableEventStream::reconstruct):

LayoutTests:

Reviewed by Mark Hahnenberg.

Added one version of the test (dfg-int32-to-double-on-set-local-and-exit) that is based
exactly on Gabor's original test, and another that ought to fail even if I fix other bugs
in the future (see ​https://bugs.webkit.org/show_bug.cgi?id=109511).

• fast/js/dfg-int32-to-double-on-set-local-and-exit-expected.txt: Added.
• fast/js/dfg-int32-to-double-on-set-local-and-exit.html: Added.
• fast/js/dfg-int32-to-double-on-set-local-and-sometimes-exit-expected.txt: Added.
• fast/js/dfg-int32-to-double-on-set-local-and-sometimes-exit.html: Added.
• fast/js/script-tests/dfg-int32-to-double-on-set-local-and-exit.js: Added.

(checkpoint):
(func1):
(func2):
(func3):
(test):

• fast/js/script-tests/dfg-int32-to-double-on-set-local-and-sometimes-exit.js: Added.

(checkpoint):
(func1):
(func2):
(func3):
(test):

File:

• trunk/Source/JavaScriptCore/dfg/DFGVariableAccessData.h (modified) (1 diff)

trunk/Source/JavaScriptCore/dfg/DFGVariableAccessData.h

@@ -178 +178 @@
         // If the variable is not a number prediction, then this doesn't
         // make any sense.
-        if (!isNumberSpeculation(prediction()))
-            return false;
+        if (!isNumberSpeculation(prediction())) {
+            // FIXME: we may end up forcing a local in inlined argument position to be a double even
+            // if it is sometimes not even numeric, since this never signals the fact that it doesn't
+            // want doubles. https://bugs.webkit.org/show_bug.cgi?id=109511
+            return false;
+        }
         
         // If the variable is predicted to hold only doubles, then it's a