Diff
Modified: branches/safari-609-branch/Source/_javascript_Core/ChangeLog (258925 => 258926)
--- branches/safari-609-branch/Source/_javascript_Core/ChangeLog 2020-03-24 19:03:08 UTC (rev 258925)
+++ branches/safari-609-branch/Source/_javascript_Core/ChangeLog 2020-03-24 19:03:11 UTC (rev 258926)
@@ -1,4 +1,4 @@
-2020-03-17 Alan Coon <[email protected]>
+b"2020-03-24 Russell Epstein <[email protected]>\n\n Cherry-pick r258901. rdar://problem/60827028\n\n HasIndexedProperty should know about sane chain\n https://bugs.webkit.org/show_bug.cgi?id=209457\n \n Reviewed by Saam Barati.\n \n This patch makes it so HasIndexedProperty is aware of\n sane chain. This is useful because, most of the time we do an\n indexed in it is on an array. If the array has a sane chain (i.e.\n no indexed properties on it's prototypes and has the default\n prototype chain) then we can just test for the index being a hole.\n \n Note, we could also just convert OOB indices into false but that\n should happen in another patch.\n https://bugs.webkit.org/show_bug.cgi?id=209456\n \n I didn't add any tests because it turns out we already have a ton.\n I know this because I broke most of them repeatedly... >.>\n \n * dfg/DFGAbstractInterpreterInlines.h:\n (JSC::DFG::Abst
ractInterpreter<AbstractStateType>::executeEffects):\n * dfg/DFGClobberize.h:\n (JSC::DFG::clobberize):\n * dfg/DFGFixupPhase.cpp:\n (JSC::DFG::FixupPhase::fixupNode):\n (JSC::DFG::FixupPhase::setSaneChainIfPossible):\n (JSC::DFG::FixupPhase::convertToHasIndexedProperty):\n * dfg/DFGNodeType.h:\n * dfg/DFGSpeculativeJIT.cpp:\n (JSC::DFG::SpeculativeJIT::compileHasIndexedProperty):\n * ftl/FTLLowerDFGToB3.cpp:\n (JSC::FTL::DFG::LowerDFGToB3::compileHasIndexedProperty):\n (JSC::FTL::DFG::LowerDFGToB3::speculateAndJump):\n * jit/AssemblyHelpers.h:\n (JSC::AssemblyHelpers::isEmpty):\n \n \n git-svn-id: https://svn.webkit.org/repository/webkit/trunk@258901 268f45cc-cd09-0410-ab3c-d52691b4dbfc\n\n 2020-03-23 Keith Miller <[email protected]>\n\n HasIndexedProperty should know about sane chain\n https://bugs.webkit.org/show_bug.cgi?id=209457\n\n Reviewed by Saam Barati.\n\n T
his patch makes it so HasIndexedProperty is aware of\n sane chain. This is useful because, most of the time we do an\n indexed in it is on an array. If the array has a sane chain (i.e.\n no indexed properties on it's prototypes and has the default\n prototype chain) then we can just test for the index being a hole.\n\n Note, we could also just convert OOB indices into false but that\n should happen in another patch.\n https://bugs.webkit.org/show_bug.cgi?id=209456\n\n I didn't add any tests because it turns out we already have a ton.\n I know this because I broke most of them repeatedly... >.>\n\n * dfg/DFGAbstractInterpreterInlines.h:\n (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):\n * dfg/DFGClobberize.h:\n (JSC::DFG::clobberize):\n * dfg/DFGFixupPhase.cpp:\n (JSC::DFG::FixupPhase::fix
upNode):\n (JSC::DFG::FixupPhase::setSaneChainIfPossible):\n (JSC::DFG::FixupPhase::convertToHasIndexedProperty):\n * dfg/DFGNodeType.h:\n * dfg/DFGSpeculativeJIT.cpp:\n (JSC::DFG::SpeculativeJIT::compileHasIndexedProperty):\n * ftl/FTLLowerDFGToB3.cpp:\n (JSC::FTL::DFG::LowerDFGToB3::compileHasIndexedProperty):\n (JSC::FTL::DFG::LowerDFGToB3::speculateAndJump):\n * jit/AssemblyHelpers.h:\n (JSC::AssemblyHelpers::isEmpty):\n\n"2020-03-17 Alan Coon <[email protected]>
Apply patch. rdar://problem/60396286
Modified: branches/safari-609-branch/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (258925 => 258926)
--- branches/safari-609-branch/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2020-03-24 19:03:08 UTC (rev 258925)
+++ branches/safari-609-branch/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2020-03-24 19:03:11 UTC (rev 258926)
@@ -3888,7 +3888,9 @@
case Array::Double:
case Array::Contiguous:
case Array::ArrayStorage: {
- break;
+ if (mode.isInBounds())
+ break;
+ FALLTHROUGH;
}
default: {
clobberWorld();
Modified: branches/safari-609-branch/Source/_javascript_Core/dfg/DFGClobberize.h (258925 => 258926)
--- branches/safari-609-branch/Source/_javascript_Core/dfg/DFGClobberize.h 2020-03-24 19:03:08 UTC (rev 258925)
+++ branches/safari-609-branch/Source/_javascript_Core/dfg/DFGClobberize.h 2020-03-24 19:03:11 UTC (rev 258926)
@@ -324,8 +324,7 @@
def(HeapLocation(HasIndexedPropertyLoc, IndexedInt32Properties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
return;
}
- read(Heap);
- return;
+ break;
}
case Array::Double: {
@@ -335,8 +334,7 @@
def(HeapLocation(HasIndexedPropertyLoc, IndexedDoubleProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
return;
}
- read(Heap);
- return;
+ break;
}
case Array::Contiguous: {
@@ -346,8 +344,7 @@
def(HeapLocation(HasIndexedPropertyLoc, IndexedContiguousProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
return;
}
- read(Heap);
- return;
+ break;
}
case Array::ArrayStorage: {
@@ -356,17 +353,14 @@
read(IndexedArrayStorageProperties);
return;
}
- read(Heap);
- return;
+ break;
}
- default: {
- read(World);
- write(Heap);
- return;
+ default:
+ break;
}
- }
- RELEASE_ASSERT_NOT_REACHED();
+ read(World);
+ write(Heap);
return;
}
Modified: branches/safari-609-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (258925 => 258926)
--- branches/safari-609-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-03-24 19:03:08 UTC (rev 258925)
+++ branches/safari-609-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-03-24 19:03:11 UTC (rev 258926)
@@ -957,18 +957,8 @@
break;
}
- if (canDoSaneChain) {
- JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
- Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure(vm());
- Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure(vm());
- if (arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
- && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
- && globalObject->arrayPrototypeChainIsSane()) {
- m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
- m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
- node->setArrayMode(arrayMode.withSpeculation(Array::SaneChain));
- }
- }
+ if (canDoSaneChain)
+ setSaneChainIfPossible(node);
}
break;
@@ -1970,6 +1960,12 @@
blessArrayOperation(m_graph.varArgChild(node, 0), m_graph.varArgChild(node, 1), m_graph.varArgChild(node, 2));
fixEdge<CellUse>(m_graph.varArgChild(node, 0));
fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
+
+ ArrayMode arrayMode = node->arrayMode();
+ // FIXME: OutOfBounds shouldn't preclude going sane chain because OOB is just false and cannot have effects.
+ // See: https://bugs.webkit.org/show_bug.cgi?id=209456
+ if (arrayMode.isJSArrayWithOriginalStructure() && arrayMode.speculation() == Array::InBounds)
+ setSaneChainIfPossible(node);
break;
}
case GetDirectPname: {
@@ -3280,6 +3276,21 @@
m_indexInBlock, SpecNone, GetIndexedPropertyStorage, origin,
OpInfo(arrayMode.asWord()), Edge(array, KnownCellUse));
}
+
+ void setSaneChainIfPossible(Node* node)
+ {
+ JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
+ Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure(vm());
+ Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure(vm());
+ if (arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
+ && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
+ && globalObject->arrayPrototypeChainIsSane()) {
+ m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
+ m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
+ node->setArrayMode(node->arrayMode().withSpeculation(Array::SaneChain));
+ node->clearFlags(NodeMustGenerate);
+ }
+ }
void blessArrayOperation(Edge base, Edge index, Edge& storageChild)
{
@@ -3632,7 +3643,6 @@
void convertToHasIndexedProperty(Node* node)
{
node->setOp(HasIndexedProperty);
- node->clearFlags(NodeMustGenerate);
{
unsigned firstChild = m_graph.m_varArgChildren.size();
@@ -3640,7 +3650,7 @@
m_graph.m_varArgChildren.append(node->child1());
m_graph.m_varArgChildren.append(node->child2());
m_graph.m_varArgChildren.append(Edge());
- node->mergeFlags(NodeHasVarArgs);
+ node->setFlags(defaultFlags(HasIndexedProperty));
node->children = AdjacencyList(AdjacencyList::Variable, firstChild, numChildren);
}
@@ -3653,6 +3663,11 @@
node->setInternalMethodType(PropertySlot::InternalMethodType::HasProperty);
blessArrayOperation(m_graph.varArgChild(node, 0), m_graph.varArgChild(node, 1), m_graph.varArgChild(node, 2));
+ auto arrayMode = node->arrayMode();
+ // FIXME: OutOfBounds shouldn't preclude going sane chain because OOB is just false and cannot have effects.
+ // See: https://bugs.webkit.org/show_bug.cgi?id=209456
+ if (arrayMode.isJSArrayWithOriginalStructure() && arrayMode.speculation() == Array::InBounds)
+ setSaneChainIfPossible(node);
fixEdge<CellUse>(m_graph.varArgChild(node, 0));
fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
Modified: branches/safari-609-branch/Source/_javascript_Core/dfg/DFGNodeType.h (258925 => 258926)
--- branches/safari-609-branch/Source/_javascript_Core/dfg/DFGNodeType.h 2020-03-24 19:03:08 UTC (rev 258925)
+++ branches/safari-609-branch/Source/_javascript_Core/dfg/DFGNodeType.h 2020-03-24 19:03:11 UTC (rev 258926)
@@ -474,7 +474,8 @@
\
/* For-in enumeration opcodes */\
macro(GetEnumerableLength, NodeMustGenerate | NodeResultJS) \
- macro(HasIndexedProperty, NodeResultBoolean | NodeHasVarArgs) \
+ /* Must generate because of Proxies on the prototype chain */ \
+ macro(HasIndexedProperty, NodeMustGenerate | NodeResultBoolean | NodeHasVarArgs) \
macro(HasStructureProperty, NodeResultBoolean) \
macro(HasGenericProperty, NodeResultBoolean) \
macro(GetDirectPname, NodeMustGenerate | NodeHasVarArgs | NodeResultJS) \
Modified: branches/safari-609-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (258925 => 258926)
--- branches/safari-609-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2020-03-24 19:03:08 UTC (rev 258925)
+++ branches/safari-609-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2020-03-24 19:03:11 UTC (rev 258926)
@@ -13545,6 +13545,7 @@
GPRReg scratchGPR = scratch.gpr();
MacroAssembler::Jump outOfBounds = m_jit.branch32(MacroAssembler::AboveOrEqual, indexGPR, MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()));
+
if (mode.isInBounds())
speculationCheck(OutOfBounds, JSValueRegs(), nullptr, outOfBounds);
else
@@ -13552,11 +13553,20 @@
#if USE(JSVALUE64)
m_jit.load64(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight), scratchGPR);
- slowCases.append(m_jit.branchIfEmpty(scratchGPR));
#else
m_jit.load32(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight, OBJECT_OFFSETOF(JSValue, u.asBits.tag)), scratchGPR);
- slowCases.append(m_jit.branchIfEmpty(scratchGPR));
#endif
+
+ if (mode.isSaneChain()) {
+ m_jit.isEmpty(scratchGPR, resultGPR);
+ break;
+ }
+
+ MacroAssembler::Jump isHole = m_jit.branchIfEmpty(scratchGPR);
+ if (!mode.isInBounds())
+ slowCases.append(isHole);
+ else
+ speculationCheck(LoadFromHole, JSValueRegs(), nullptr, isHole);
m_jit.move(TrustedImm32(1), resultGPR);
break;
}
@@ -13568,6 +13578,7 @@
GPRReg storageGPR = storage.gpr();
MacroAssembler::Jump outOfBounds = m_jit.branch32(MacroAssembler::AboveOrEqual, indexGPR, MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()));
+
if (mode.isInBounds())
speculationCheck(OutOfBounds, JSValueRegs(), nullptr, outOfBounds);
else
@@ -13574,7 +13585,17 @@
slowCases.append(outOfBounds);
m_jit.loadDouble(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight), scratchFPR);
- slowCases.append(m_jit.branchIfNaN(scratchFPR));
+
+ if (mode.isSaneChain()) {
+ m_jit.compareDouble(MacroAssembler::DoubleEqualAndOrdered, scratchFPR, scratchFPR, resultGPR);
+ break;
+ }
+
+ MacroAssembler::Jump isHole = m_jit.branchIfNaN(scratchFPR);
+ if (!mode.isInBounds())
+ slowCases.append(isHole);
+ else
+ speculationCheck(LoadFromHole, JSValueRegs(), nullptr, isHole);
m_jit.move(TrustedImm32(1), resultGPR);
break;
}
@@ -13594,11 +13615,20 @@
#if USE(JSVALUE64)
m_jit.load64(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight, ArrayStorage::vectorOffset()), scratchGPR);
- slowCases.append(m_jit.branchIfEmpty(scratchGPR));
#else
m_jit.load32(MacroAssembler::BaseIndex(storageGPR, indexGPR, MacroAssembler::TimesEight, ArrayStorage::vectorOffset() + OBJECT_OFFSETOF(JSValue, u.asBits.tag)), scratchGPR);
- slowCases.append(m_jit.branchIfEmpty(scratchGPR));
#endif
+
+ if (mode.isSaneChain()) {
+ m_jit.isEmpty(scratchGPR, resultGPR);
+ break;
+ }
+
+ MacroAssembler::Jump isHole = m_jit.branchIfEmpty(scratchGPR);
+ if (!mode.isInBounds() || mode.isSaneChain())
+ slowCases.append(isHole);
+ else
+ speculationCheck(LoadFromHole, JSValueRegs(), nullptr, isHole);
m_jit.move(TrustedImm32(1), resultGPR);
break;
}
Modified: branches/safari-609-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (258925 => 258926)
--- branches/safari-609-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-03-24 19:03:08 UTC (rev 258925)
+++ branches/safari-609-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-03-24 19:03:11 UTC (rev 258926)
@@ -11412,6 +11412,7 @@
JSGlobalObject* globalObject = m_graph.globalObjectFor(m_node->origin.semantic);
LValue base = lowCell(m_graph.varArgChild(m_node, 0));
LValue index = lowInt32(m_graph.varArgChild(m_node, 1));
+ ArrayMode mode = m_node->arrayMode();
switch (m_node->arrayMode().type()) {
case Array::Int32:
@@ -11419,7 +11420,7 @@
LValue storage = lowStorage(m_graph.varArgChild(m_node, 2));
LValue internalMethodType = m_out.constInt32(static_cast<int32_t>(m_node->internalMethodType()));
- IndexedAbstractHeap& heap = m_node->arrayMode().type() == Array::Int32 ?
+ IndexedAbstractHeap& heap = mode.type() == Array::Int32 ?
m_heaps.indexedInt32Properties : m_heaps.indexedContiguousProperties;
LBasicBlock slowCase = m_out.newBlock();
@@ -11426,7 +11427,7 @@
LBasicBlock continuation = m_out.newBlock();
LBasicBlock lastNext = nullptr;
- if (!m_node->arrayMode().isInBounds()) {
+ if (!mode.isInBounds()) {
LBasicBlock checkHole = m_out.newBlock();
m_out.branch(
m_out.aboveOrEqual(
@@ -11439,7 +11440,12 @@
LValue checkHoleResultValue =
m_out.notZero64(m_out.load64(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1))));
ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
- m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
+ if (mode.isSaneChain())
+ m_out.jump(continuation);
+ else if (!mode.isInBounds())
+ m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
+ else
+ speculateAndJump(continuation, LoadFromHole, noValue(), nullptr, checkHoleResultValue);
m_out.appendTo(slowCase, continuation);
ValueFromBlock slowResult = m_out.anchor(
@@ -11473,7 +11479,12 @@
LValue doubleValue = m_out.loadDouble(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1)));
LValue checkHoleResultValue = m_out.doubleEqual(doubleValue, doubleValue);
ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
- m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
+ if (mode.isSaneChain())
+ m_out.jump(continuation);
+ else if (!mode.isInBounds())
+ m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
+ else
+ speculateAndJump(continuation, LoadFromHole, noValue(), nullptr, checkHoleResultValue);
m_out.appendTo(slowCase, continuation);
ValueFromBlock slowResult = m_out.anchor(
@@ -11506,7 +11517,12 @@
LValue checkHoleResultValue =
m_out.notZero64(m_out.load64(baseIndex(m_heaps.ArrayStorage_vector, storage, index, m_graph.varArgChild(m_node, 1))));
ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
- m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
+ if (mode.isSaneChain())
+ m_out.jump(continuation);
+ else if (!mode.isInBounds())
+ m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
+ else
+ speculateAndJump(continuation, LoadFromHole, noValue(), nullptr, checkHoleResultValue);
m_out.appendTo(slowCase, continuation);
ValueFromBlock slowResult = m_out.anchor(
@@ -15888,6 +15904,13 @@
{
appendOSRExit(kind, lowValue, profile, failCondition, m_origin);
}
+
+ template<typename... Args>
+ void speculateAndJump(B3::BasicBlock* target, Args... args)
+ {
+ speculate(args...);
+ m_out.jump(target);
+ }
void terminate(ExitKind kind)
{
Modified: branches/safari-609-branch/Source/_javascript_Core/jit/AssemblyHelpers.h (258925 => 258926)
--- branches/safari-609-branch/Source/_javascript_Core/jit/AssemblyHelpers.h 2020-03-24 19:03:08 UTC (rev 258925)
+++ branches/safari-609-branch/Source/_javascript_Core/jit/AssemblyHelpers.h 2020-03-24 19:03:11 UTC (rev 258926)
@@ -985,6 +985,15 @@
Jump branchIfFunction(GPRReg cellGPR) { return branchIfType(cellGPR, JSFunctionType); }
Jump branchIfNotFunction(GPRReg cellGPR) { return branchIfNotType(cellGPR, JSFunctionType); }
+ void isEmpty(GPRReg gpr, GPRReg dst)
+ {
+#if USE(JSVALUE64)
+ test64(NonZero, gpr, TrustedImm32(-1), dst);
+#else
+ compare32(Equal, gpr, TrustedImm32(JSValue::EmptyValueTag), dst);
+#endif
+ }
+
Jump branchIfEmpty(GPRReg gpr)
{
#if USE(JSVALUE64)
