Changeset 142544 in webkit for trunk/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp

Timestamp:

Feb 11, 2013, 4:29:20 PM (12 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/DFGCSEPhase.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp

@@ -127 +127 @@
     }
     
+    Node* int32ToDoubleCSE(Node* node)
+    {
+        for (unsigned i = m_indexInBlock; i--;) {
+            Node* otherNode = m_currentBlock->at(i);
+            if (otherNode == node->child1())
+                return 0;
+            if (!otherNode->shouldGenerate())
+                continue;
+            switch (otherNode->op()) {
+            case Int32ToDouble:
+            case ForwardInt32ToDouble:
+                if (otherNode->child1() == node->child1())
+                    return otherNode;
+                break;
+            default:
+                break;
+            }
+        }
+        return 0;
+    }
+    
     Node* constantCSE(Node* node)
     {
@@ -1098 +1119 @@
         case StringCharAt:
         case StringCharCodeAt:
-        case Int32ToDouble:
         case IsUndefined:
         case IsBoolean:
@@ -1114 +1134 @@
         case CompareEqConstant:
             setReplacement(pureCSE(node));
+            break;
+            
+        case Int32ToDouble:
+        case ForwardInt32ToDouble:
+            setReplacement(int32ToDoubleCSE(node));
             break;