Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (258900 => 258901)
--- trunk/Source/_javascript_Core/ChangeLog 2020-03-24 02:44:31 UTC (rev 258900)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-03-24 03:02:28 UTC (rev 258901)
@@ -1,3 +1,40 @@
+2020-03-23 Keith Miller <keith_mil...@apple.com>
+
+ HasIndexedProperty should know about sane chain
+ https://bugs.webkit.org/show_bug.cgi?id=209457
+
+ Reviewed by Saam Barati.
+
+ This patch makes it so HasIndexedProperty is aware of
+ sane chain. This is useful because, most of the time we do an
+ indexed in it is on an array. If the array has a sane chain (i.e.
+ no indexed properties on it's prototypes and has the default
+ prototype chain) then we can just test for the index being a hole.
+
+ Note, we could also just convert OOB indices into false but that
+ should happen in another patch.
+ https://bugs.webkit.org/show_bug.cgi?id=209456
+
+ I didn't add any tests because it turns out we already have a ton.
+ I know this because I broke most of them repeatedly... >.>
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ (JSC::DFG::FixupPhase::setSaneChainIfPossible):
+ (JSC::DFG::FixupPhase::convertToHasIndexedProperty):
+ * dfg/DFGNodeType.h:
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileHasIndexedProperty):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileHasIndexedProperty):
+ (JSC::FTL::DFG::LowerDFGToB3::speculateAndJump):
+ * jit/AssemblyHelpers.h:
+ (JSC::AssemblyHelpers::isEmpty):
+
2020-03-23 Yusuke Suzuki <ysuz...@apple.com>
[JSC] Caller of Delete IC should emit write-barrier onto owner
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (258900 => 258901)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2020-03-24 02:44:31 UTC (rev 258900)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2020-03-24 03:02:28 UTC (rev 258901)
@@ -3918,7 +3918,9 @@
case Array::Double:
case Array::Contiguous:
case Array::ArrayStorage: {
- break;
+ if (mode.isInBounds())
+ break;
+ FALLTHROUGH;
}
default: {
clobberWorld();
Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (258900 => 258901)
--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2020-03-24 02:44:31 UTC (rev 258900)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2020-03-24 03:02:28 UTC (rev 258901)
@@ -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: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (258900 => 258901)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-03-24 02:44:31 UTC (rev 258900)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-03-24 03:02:28 UTC (rev 258901)
@@ -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;
@@ -2029,6 +2019,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: {
@@ -3342,6 +3338,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)
{
@@ -3694,7 +3705,6 @@
void convertToHasIndexedProperty(Node* node)
{
node->setOp(HasIndexedProperty);
- node->clearFlags(NodeMustGenerate);
{
unsigned firstChild = m_graph.m_varArgChildren.size();
@@ -3702,7 +3712,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);
}
@@ -3715,6 +3725,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: trunk/Source/_javascript_Core/dfg/DFGNodeType.h (258900 => 258901)
--- trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2020-03-24 02:44:31 UTC (rev 258900)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2020-03-24 03:02:28 UTC (rev 258901)
@@ -483,7 +483,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: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (258900 => 258901)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2020-03-24 02:44:31 UTC (rev 258900)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2020-03-24 03:02:28 UTC (rev 258901)
@@ -13580,6 +13580,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
@@ -13587,11 +13588,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;
}
@@ -13603,6 +13613,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
@@ -13609,7 +13620,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;
}
@@ -13629,11 +13650,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: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (258900 => 258901)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-03-24 02:44:31 UTC (rev 258900)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-03-24 03:02:28 UTC (rev 258901)
@@ -11593,6 +11593,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:
@@ -11600,7 +11601,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();
@@ -11607,7 +11608,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(
@@ -11620,7 +11621,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(
@@ -11654,7 +11660,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(
@@ -11687,7 +11698,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(
@@ -16126,6 +16142,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: trunk/Source/_javascript_Core/jit/AssemblyHelpers.h (258900 => 258901)
--- trunk/Source/_javascript_Core/jit/AssemblyHelpers.h 2020-03-24 02:44:31 UTC (rev 258900)
+++ trunk/Source/_javascript_Core/jit/AssemblyHelpers.h 2020-03-24 03:02:28 UTC (rev 258901)
@@ -961,6 +961,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)
