Merged r241228 - Nodes that rely on being dominated by CheckInBounds …

…should have a child edge to it https://bugs.webkit.org/show_bug.cgi?id=194334 <rdar://problem/47844327> Reviewed by Mark Lam. JSTests: * stress/check-in-bounds-should-be-a-child-use.js: Added. (func): Source/JavaScriptCore: * dfg/DFGAbstractInterpreterInlines.h: (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects): * dfg/DFGArgumentsEliminationPhase.cpp: * dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::parseBlock): * dfg/DFGClobberize.h: (JSC::DFG::clobberize): * dfg/DFGConstantFoldingPhase.cpp: (JSC::DFG::ConstantFoldingPhase::foldConstants): * dfg/DFGFixupPhase.cpp: (JSC::DFG::FixupPhase::fixupNode): (JSC::DFG::FixupPhase::convertToHasIndexedProperty): * dfg/DFGIntegerCheckCombiningPhase.cpp: (JSC::DFG::IntegerCheckCombiningPhase::handleBlock): * dfg/DFGIntegerRangeOptimizationPhase.cpp: * dfg/DFGNodeType.h: * dfg/DFGSSALoweringPhase.cpp: (JSC::DFG::SSALoweringPhase::lowerBoundsCheck): * dfg/DFGSpeculativeJIT.cpp: (JSC::DFG::SpeculativeJIT::compileHasIndexedProperty): * ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileCheckInBounds): (JSC::FTL::DFG::LowerDFGToB3::compileHasIndexedProperty):
WebKit · Feb 24, 2019 · eed44b4 · eed44b4
1 parent 79f9a90
commit eed44b4
Show file tree

Hide file tree

Showing 15 changed files with 143 additions and 63 deletions.
diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
@@ -1,3 +1,14 @@
+2019-02-08  Saam barati  <sbarati@apple.com>
+
+        Nodes that rely on being dominated by CheckInBounds should have a child edge to it
+        https://bugs.webkit.org/show_bug.cgi?id=194334
+        <rdar://problem/47844327>
+
+        Reviewed by Mark Lam.
+
+        * stress/check-in-bounds-should-be-a-child-use.js: Added.
+        (func):
+
 2018-12-19  Mark Lam  <mark.lam@apple.com>
 
         JSPropertyNameEnumerator should cache the iterated object's structure only after getting its property names.

diff --git a/JSTests/stress/check-in-bounds-should-be-a-child-use.js b/JSTests/stress/check-in-bounds-should-be-a-child-use.js
@@ -0,0 +1,16 @@
+//@ runDefault("--useConcurrentJIT=0", "--thresholdForFTLOptimizeAfterWarmUp=100")
+
+const hello = [1337,1337,1337,1337];
+const arr = [1337,1337];
+
+function func(arg) {
+    for (let p in arg) {
+        arg.a = 42;
+        const val = arg[-698666199];
+    }
+}
+
+for (let i = 0; i < 10000; ++i) {
+    const a = func(arr);
+    const b = func(1337);
+}
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,35 @@
+2019-02-08  Saam barati  <sbarati@apple.com>
+
+        Nodes that rely on being dominated by CheckInBounds should have a child edge to it
+        https://bugs.webkit.org/show_bug.cgi?id=194334
+        <rdar://problem/47844327>
+
+        Reviewed by Mark Lam.
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGArgumentsEliminationPhase.cpp:
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGConstantFoldingPhase.cpp:
+        (JSC::DFG::ConstantFoldingPhase::foldConstants):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        (JSC::DFG::FixupPhase::convertToHasIndexedProperty):
+        * dfg/DFGIntegerCheckCombiningPhase.cpp:
+        (JSC::DFG::IntegerCheckCombiningPhase::handleBlock):
+        * dfg/DFGIntegerRangeOptimizationPhase.cpp:
+        * dfg/DFGNodeType.h:
+        * dfg/DFGSSALoweringPhase.cpp:
+        (JSC::DFG::SSALoweringPhase::lowerBoundsCheck):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileHasIndexedProperty):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileCheckInBounds):
+        (JSC::FTL::DFG::LowerDFGToB3::compileHasIndexedProperty):
+
 2019-02-18  Mark Lam  <mark.lam@apple.com>
 
         Fix DFG doesGC() for CompareEq/Less/LessEq/Greater/GreaterEq and CompareStrictEq nodes.

diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
@@ -3370,11 +3370,16 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
     case CheckInBounds: {
         JSValue left = forNode(node->child1()).value();
         JSValue right = forNode(node->child2()).value();
-        if (left && right && left.isInt32() && right.isInt32()
-            && static_cast<uint32_t>(left.asInt32()) < static_cast<uint32_t>(right.asInt32())) {
+        if (left && right && left.isInt32() && right.isInt32() && static_cast<uint32_t>(left.asInt32()) < static_cast<uint32_t>(right.asInt32()))
             m_state.setFoundConstants(true);
-            break;
-        }
+
+        // We claim we result in Int32. It's not really important what our result is (though we
+        // don't want to claim we may result in the empty value), other nodes with data flow edges
+        // to us just do that to maintain the invariant that they can't be hoisted higher than us.
+        // So we just arbitrarily pick Int32. In some ways, StorageResult may be the more correct
+        // thing to do here. We pick NodeResultJS because it makes converting this to an identity
+        // easier.
+        setNonCellTypeForNode(node, SpecInt32Only);
         break;
     }
 

diff --git a/Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp b/Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp
@@ -767,14 +767,15 @@ class ArgumentsEliminationPhase : public Phase {
                                 arg += inlineCallFrame->stackOffset;
                             data = m_graph.m_stackAccessData.add(arg, FlushedJSValue);
 
+                            Node* check = nullptr;
                             if (!inlineCallFrame || inlineCallFrame->isVarargs()) {
-                                insertionSet.insertNode(
+                                check = insertionSet.insertNode(
                                     nodeIndex, SpecNone, CheckInBounds, node->origin,
                                     m_graph.varArgChild(node, 1), Edge(getArrayLength(candidate), Int32Use));
                             }
 
                             result = insertionSet.insertNode(
-                                nodeIndex, node->prediction(), GetStack, node->origin, OpInfo(data));
+                                nodeIndex, node->prediction(), GetStack, node->origin, OpInfo(data), Edge(check, UntypedUse));
                         }
                     }
 

diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -6714,7 +6714,10 @@ void ByteCodeParser::parseBlock(unsigned limit)
             Node* base = get(VirtualRegister(currentInstruction[2].u.operand));
             ArrayMode arrayMode = getArrayMode(arrayProfileFor<OpHasIndexedPropertyShape>(currentInstruction), Array::Read);
             Node* property = get(VirtualRegister(currentInstruction[3].u.operand));
-            Node* hasIterableProperty = addToGraph(HasIndexedProperty, OpInfo(arrayMode.asWord()), OpInfo(static_cast<uint32_t>(PropertySlot::InternalMethodType::GetOwnProperty)), base, property);
+            addVarArgChild(base);
+            addVarArgChild(property);
+            addVarArgChild(nullptr);
+            Node* hasIterableProperty = addToGraph(Node::VarArg, HasIndexedProperty, OpInfo(arrayMode.asWord()), OpInfo(static_cast<uint32_t>(PropertySlot::InternalMethodType::GetOwnProperty)));
             set(VirtualRegister(currentInstruction[1].u.operand), hasIterableProperty);
             NEXT_OPCODE(op_has_indexed_property);
         }

diff --git a/Source/JavaScriptCore/dfg/DFGClobberize.h b/Source/JavaScriptCore/dfg/DFGClobberize.h
@@ -306,7 +306,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
             if (mode.isInBounds()) {
                 read(Butterfly_publicLength);
                 read(IndexedInt32Properties);
-                def(HeapLocation(HasIndexedPropertyLoc, IndexedInt32Properties, node->child1(), node->child2()), LazyNode(node));
+                def(HeapLocation(HasIndexedPropertyLoc, IndexedInt32Properties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
                 return;
             }
             read(Heap);
@@ -317,7 +317,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
             if (mode.isInBounds()) {
                 read(Butterfly_publicLength);
                 read(IndexedDoubleProperties);
-                def(HeapLocation(HasIndexedPropertyLoc, IndexedDoubleProperties, node->child1(), node->child2()), LazyNode(node));
+                def(HeapLocation(HasIndexedPropertyLoc, IndexedDoubleProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
                 return;
             }
             read(Heap);
@@ -328,7 +328,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
             if (mode.isInBounds()) {
                 read(Butterfly_publicLength);
                 read(IndexedContiguousProperties);
-                def(HeapLocation(HasIndexedPropertyLoc, IndexedContiguousProperties, node->child1(), node->child2()), LazyNode(node));
+                def(HeapLocation(HasIndexedPropertyLoc, IndexedContiguousProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
                 return;
             }
             read(Heap);

diff --git a/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp b/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp
@@ -341,7 +341,9 @@ class ConstantFoldingPhase : public Phase {
                 JSValue right = m_state.forNode(node->child2()).value();
                 if (left && right && left.isInt32() && right.isInt32()
                     && static_cast<uint32_t>(left.asInt32()) < static_cast<uint32_t>(right.asInt32())) {
-                    node->remove(m_graph);
+
+                    Node* zero = m_insertionSet.insertConstant(indexInBlock, node->origin, jsNumber(0));
+                    node->convertToIdentityOn(zero);
                     eliminated = true;
                     break;
                 }
@@ -410,10 +412,11 @@ class ConstantFoldingPhase : public Phase {
 
                 Node* length = emitCodeToGetArgumentsArrayLength(
                     m_insertionSet, arguments, indexInBlock, node->origin);
-                m_insertionSet.insertNode(
+                Node* check = m_insertionSet.insertNode(
                     indexInBlock, SpecNone, CheckInBounds, node->origin,
                     node->child2(), Edge(length, Int32Use));
                 node->convertToGetStack(data);
+                node->child1() = Edge(check, UntypedUse);
                 eliminated = true;
                 break;
             }

diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -1698,13 +1698,13 @@ class FixupPhase : public Phase {
             node->setArrayMode(
                 node->arrayMode().refine(
                     m_graph, node,
-                    node->child1()->prediction(),
-                    node->child2()->prediction(),
+                    m_graph.varArgChild(node, 0)->prediction(),
+                    m_graph.varArgChild(node, 1)->prediction(),
                     SpecNone));
 
-            blessArrayOperation(node->child1(), node->child2(), node->child3());
-            fixEdge<CellUse>(node->child1());
-            fixEdge<KnownInt32Use>(node->child2());
+            blessArrayOperation(m_graph.varArgChild(node, 0), m_graph.varArgChild(node, 1), m_graph.varArgChild(node, 2));
+            fixEdge<CellUse>(m_graph.varArgChild(node, 0));
+            fixEdge<KnownInt32Use>(m_graph.varArgChild(node, 1));
             break;
         }
         case GetDirectPname: {
@@ -3322,18 +3322,29 @@ class FixupPhase : public Phase {
     {
         node->setOp(HasIndexedProperty);
         node->clearFlags(NodeMustGenerate);
+
+        {
+            unsigned firstChild = m_graph.m_varArgChildren.size();
+            unsigned numChildren = 3;
+            m_graph.m_varArgChildren.append(node->child1());
+            m_graph.m_varArgChildren.append(node->child2());
+            m_graph.m_varArgChildren.append(Edge());
+            node->mergeFlags(NodeHasVarArgs);
+            node->children = AdjacencyList(AdjacencyList::Variable, firstChild, numChildren);
+        }
+
         node->setArrayMode(
             node->arrayMode().refine(
                 m_graph, node,
-                node->child1()->prediction(),
-                node->child2()->prediction(),
+                m_graph.varArgChild(node, 0)->prediction(),
+                m_graph.varArgChild(node, 1)->prediction(),
                 SpecNone));
         node->setInternalMethodType(PropertySlot::InternalMethodType::HasProperty);
 
-        blessArrayOperation(node->child1(), node->child2(), node->child3());
+        blessArrayOperation(m_graph.varArgChild(node, 0), m_graph.varArgChild(node, 1), m_graph.varArgChild(node, 2));
 
-        fixEdge<CellUse>(node->child1());
-        fixEdge<Int32Use>(node->child2());
+        fixEdge<CellUse>(m_graph.varArgChild(node, 0));
+        fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
     }
 
     void fixupNormalizeMapKey(Node* node)

diff --git a/Source/JavaScriptCore/dfg/DFGIntegerCheckCombiningPhase.cpp b/Source/JavaScriptCore/dfg/DFGIntegerCheckCombiningPhase.cpp
@@ -150,6 +150,7 @@ class IntegerCheckCombiningPhase : public Phase {
         CodeOrigin m_maxOrigin;
         unsigned m_count { 0 }; // If this is zero then the bounds won't necessarily make sense.
         bool m_hoisted { false };
+        Node* m_dependency { nullptr };
     };
 
     IntegerCheckCombiningPhase(Graph& graph)
@@ -257,14 +258,15 @@ class IntegerCheckCombiningPhase : public Phase {
                             Arith::Unchecked);
                     }
 
+                    Node* minCheck = nullptr;
                     if (minNode) {
-                        m_insertionSet.insertNode(
+                        minCheck = m_insertionSet.insertNode(
                             nodeIndex, SpecNone, CheckInBounds, node->origin,
                             Edge(minNode, Int32Use), Edge(data.m_key.m_key, Int32Use));
                     }
-                    m_insertionSet.insertNode(
+                    m_map[data.m_key].m_dependency = m_insertionSet.insertNode(
                         nodeIndex, SpecNone, CheckInBounds, node->origin,
-                        Edge(maxNode, Int32Use), Edge(data.m_key.m_key, Int32Use));
+                        Edge(maxNode, Int32Use), Edge(data.m_key.m_key, Int32Use), Edge(minCheck, UntypedUse));
                     break;
                 }
 
@@ -284,7 +286,7 @@ class IntegerCheckCombiningPhase : public Phase {
                 break;
 
             case ArrayBounds:
-                node->remove(m_graph);
+                node->convertToIdentityOn(m_map[data.m_key].m_dependency);
                 m_changed = true;
                 break;
 

diff --git a/Source/JavaScriptCore/dfg/DFGIntegerRangeOptimizationPhase.cpp b/Source/JavaScriptCore/dfg/DFGIntegerRangeOptimizationPhase.cpp
@@ -1010,16 +1010,8 @@ class IntegerRangeOptimizationPhase : public Phase {
         ASSERT(m_graph.m_form == SSA);
 
         // Before we do anything, make sure that we have a zero constant at the top.
-        for (Node* node : *m_graph.block(0)) {
-            if (node->isInt32Constant() && !node->asInt32()) {
-                m_zero = node;
-                break;
-            }
-        }
-        if (!m_zero) {
-            m_zero = m_insertionSet.insertConstant(0, m_graph.block(0)->at(0)->origin, jsNumber(0));
-            m_insertionSet.execute(m_graph.block(0));
-        }
+        m_zero = m_insertionSet.insertConstant(0, m_graph.block(0)->at(0)->origin, jsNumber(0));
+        m_insertionSet.execute(m_graph.block(0));
 
         if (DFGIntegerRangeOptimizationPhaseInternal::verbose) {
             dataLog("Graph before integer range optimization:\n");
@@ -1330,7 +1322,7 @@ class IntegerRangeOptimizationPhase : public Phase {
 
                     if (nonNegative && lessThanLength) {
                         executeNode(block->at(nodeIndex));
-                        node->remove(m_graph);
+                        node->convertToIdentityOn(m_zero);
                         changed = true;
                     }
                     break;

diff --git a/Source/JavaScriptCore/dfg/DFGNodeType.h b/Source/JavaScriptCore/dfg/DFGNodeType.h
@@ -250,7 +250,7 @@ namespace JSC { namespace DFG {
     macro(CheckNotEmpty, NodeMustGenerate) \
     macro(AssertNotEmpty, NodeMustGenerate) \
     macro(CheckBadCell, NodeMustGenerate) \
-    macro(CheckInBounds, NodeMustGenerate) \
+    macro(CheckInBounds, NodeMustGenerate | NodeResultJS) \
     macro(CheckStringIdent, NodeMustGenerate) \
     macro(CheckTypeInfoFlags, NodeMustGenerate) /* Takes an OpInfo with the flags you want to test are set */\
     macro(CheckSubClass, NodeMustGenerate) \
@@ -441,7 +441,7 @@ namespace JSC { namespace DFG {
     \
     /* For-in enumeration opcodes */\
     macro(GetEnumerableLength, NodeMustGenerate | NodeResultJS) \
-    macro(HasIndexedProperty, NodeResultBoolean) \
+    macro(HasIndexedProperty, NodeResultBoolean | NodeHasVarArgs) \
     macro(HasStructureProperty, NodeResultBoolean) \
     macro(HasGenericProperty, NodeResultBoolean) \
     macro(GetDirectPname, NodeMustGenerate | NodeHasVarArgs | NodeResultJS) \

diff --git a/Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp b/Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp
@@ -130,10 +130,15 @@ class SSALoweringPhase : public Phase {
 
         Node* length = m_insertionSet.insertNode(
             m_nodeIndex, SpecInt32Only, op, m_node->origin,
-            OpInfo(m_node->arrayMode().asWord()), base, storage);
-        m_insertionSet.insertNode(
+            OpInfo(m_node->arrayMode().asWord()), Edge(base.node(), KnownCellUse), storage);
+        Node* checkInBounds = m_insertionSet.insertNode(
             m_nodeIndex, SpecInt32Only, CheckInBounds, m_node->origin,
             index, Edge(length, KnownInt32Use));
+
+        AdjacencyList adjacencyList = m_graph.copyVarargChildren(m_node);
+        m_graph.m_varArgChildren.append(Edge(checkInBounds, UntypedUse));
+        adjacencyList.setNumChildren(adjacencyList.numChildren() + 1);
+        m_node->children = adjacencyList;
         return true;
     }
 

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -12808,8 +12808,8 @@ void SpeculativeJIT::compileAllocateNewArrayWithSize(JSGlobalObject* globalObjec
 
 void SpeculativeJIT::compileHasIndexedProperty(Node* node)
 {
-    SpeculateCellOperand base(this, node->child1());
-    SpeculateStrictInt32Operand index(this, node->child2());
+    SpeculateCellOperand base(this, m_graph.varArgChild(node, 0));
+    SpeculateStrictInt32Operand index(this, m_graph.varArgChild(node, 1));
     GPRTemporary result(this);
 
     GPRReg baseGPR = base.gpr();
@@ -12821,8 +12821,8 @@ void SpeculativeJIT::compileHasIndexedProperty(Node* node)
     switch (mode.type()) {
     case Array::Int32:
     case Array::Contiguous: {
-        ASSERT(!!node->child3());
-        StorageOperand storage(this, node->child3());
+        ASSERT(!!m_graph.varArgChild(node, 2));
+        StorageOperand storage(this, m_graph.varArgChild(node, 2));
         GPRTemporary scratch(this);
 
         GPRReg storageGPR = storage.gpr();
@@ -12845,8 +12845,8 @@ void SpeculativeJIT::compileHasIndexedProperty(Node* node)
         break;
     }
     case Array::Double: {
-        ASSERT(!!node->child3());
-        StorageOperand storage(this, node->child3());
+        ASSERT(!!m_graph.varArgChild(node, 2));
+        StorageOperand storage(this, m_graph.varArgChild(node, 2));
         FPRTemporary scratch(this);
         FPRReg scratchFPR = scratch.fpr();
         GPRReg storageGPR = storage.gpr();
@@ -12863,8 +12863,8 @@ void SpeculativeJIT::compileHasIndexedProperty(Node* node)
         break;
     }
     case Array::ArrayStorage: {
-        ASSERT(!!node->child3());
-        StorageOperand storage(this, node->child3());
+        ASSERT(!!m_graph.varArgChild(node, 2));
+        StorageOperand storage(this, m_graph.varArgChild(node, 2));
         GPRTemporary scratch(this);
 
         GPRReg storageGPR = storage.gpr();