Diff
Modified: trunk/JSTests/ChangeLog (261711 => 261712)
--- trunk/JSTests/ChangeLog 2020-05-14 21:50:41 UTC (rev 261711)
+++ trunk/JSTests/ChangeLog 2020-05-14 22:01:50 UTC (rev 261712)
@@ -1,3 +1,13 @@
+2020-05-14 Keith Miller <[email protected]>
+
+ GetArrayLength should be "blessed" during Fixup phase in the DFG
+ https://bugs.webkit.org/show_bug.cgi?id=211540
+
+ Reviewed by Saam Barati.
+
+ * stress/get-array-length-node-should-be-blessed-in-fixup.js: Added.
+ (foo):
+
2020-05-13 Keith Miller <[email protected]>
iteration bytecodes need to handle osr exiting from inlined getter frames
Added: trunk/JSTests/stress/get-array-length-node-should-be-blessed-in-fixup.js (0 => 261712)
--- trunk/JSTests/stress/get-array-length-node-should-be-blessed-in-fixup.js (rev 0)
+++ trunk/JSTests/stress/get-array-length-node-should-be-blessed-in-fixup.js 2020-05-14 22:01:50 UTC (rev 261712)
@@ -0,0 +1,8 @@
+function foo() {
+ for (let i=0; i<10000; i++) {}
+ for (const q of Array.prototype) {}
+}
+
+for (let i=0; i<1000; i++) {
+ foo();
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (261711 => 261712)
--- trunk/Source/_javascript_Core/ChangeLog 2020-05-14 21:50:41 UTC (rev 261711)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-05-14 22:01:50 UTC (rev 261712)
@@ -1,3 +1,55 @@
+2020-05-14 Keith Miller <[email protected]>
+
+ GetArrayLength should be "blessed" during Fixup phase in the DFG
+ https://bugs.webkit.org/show_bug.cgi?id=211540
+
+ Reviewed by Saam Barati.
+
+ If we got an ArrayMode during bytecode parsing for-of that expects
+ to be configured during Fixup, then right now we will crash on
+ GetArrayLength. This fixes GetArrayLength to properly call
+ blessArrayOperation and fixes clobberize to know that
+ GetArrayLength could have a ForceExit ArrayMode briefly before
+ being cleaned up.
+
+ When blessing GetArrayLength we can now produce CheckArrays that
+ have an AnyTypedArray ArrayMode::Type. So this patch expands
+ CheckArray to properly handle that. To help with this we expand
+ branchIfType to have a starting JSType and an optional ending
+ JSType. Additionally, to prevent extra checks AI has been taught
+ to fold more ArrayModes so we should almost always avoid new
+ runtime checks.
+
+ Lastly, make sure that Undecided Arrays don't fall back to generic
+ because GetArrayLength can't be converted to...
+ GetArrayLenth. Also, GetArrayLength would previously pass it's own
+ speculation for the speculation of the index, which logically
+ doesn't make sense. So this patch adds a new constant, which is
+ SpecInt32Only, that can be passed if a DFG node doesn't have an
+ index.
+
+ * assembler/testmasm.cpp:
+ (JSC::testBranchIfType):
+ (JSC::testBranchIfNotType):
+ (JSC::run):
+ * dfg/DFGArrayMode.cpp:
+ (JSC::DFG::canBecomeGetArrayLength):
+ * dfg/DFGArrayMode.h:
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ (JSC::DFG::FixupPhase::blessArrayOperation):
+ (JSC::DFG::FixupPhase::attemptToMakeGetArrayLength):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::checkArray):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::isArrayTypeForCheckArray):
+ * jit/AssemblyHelpers.h:
+ (JSC::AssemblyHelpers::branchIfType):
+ (JSC::AssemblyHelpers::branchIfNotType):
+ * runtime/JSType.h:
+
2020-05-13 Keith Miller <[email protected]>
iteration bytecodes need to handle osr exiting from inlined getter frames
Modified: trunk/Source/_javascript_Core/assembler/testmasm.cpp (261711 => 261712)
--- trunk/Source/_javascript_Core/assembler/testmasm.cpp 2020-05-14 21:50:41 UTC (rev 261711)
+++ trunk/Source/_javascript_Core/assembler/testmasm.cpp 2020-05-14 22:01:50 UTC (rev 261712)
@@ -2161,6 +2161,74 @@
#endif
}
+static void testBranchIfType()
+{
+ using JSC::JSType;
+ struct CellLike {
+ uint32_t structureID;
+ uint8_t indexingType;
+ JSType type;
+ };
+ CHECK_EQ(JSCell::typeInfoTypeOffset(), OBJECT_OFFSETOF(CellLike, type));
+
+ auto isType = compile([] (CCallHelpers& jit) {
+ emitFunctionPrologue(jit);
+ auto isType = jit.branchIfType(GPRInfo::argumentGPR0, JSType(FirstTypedArrayType), JSType(LastTypedArrayTypeExcludingDataView));
+ jit.move(CCallHelpers::TrustedImm32(false), GPRInfo::returnValueGPR);
+ auto done = jit.jump();
+ isType.link(&jit);
+ jit.move(CCallHelpers::TrustedImm32(true), GPRInfo::returnValueGPR);
+ done.link(&jit);
+ emitFunctionEpilogue(jit);
+ jit.ret();
+ });
+
+ CellLike cell;
+ for (unsigned i = JSC::FirstTypedArrayType; i <= JSC::LastTypedArrayTypeExcludingDataView; ++i) {
+ cell.type = JSType(i);
+ CHECK_EQ(invoke<bool>(isType, &cell), true);
+ }
+
+ cell.type = JSType(LastTypedArrayType);
+ CHECK_EQ(invoke<bool>(isType, &cell), false);
+ cell.type = JSType(FirstTypedArrayType - 1);
+ CHECK_EQ(invoke<bool>(isType, &cell), false);
+}
+
+static void testBranchIfNotType()
+{
+ using JSC::JSType;
+ struct CellLike {
+ uint32_t structureID;
+ uint8_t indexingType;
+ JSType type;
+ };
+ CHECK_EQ(JSCell::typeInfoTypeOffset(), OBJECT_OFFSETOF(CellLike, type));
+
+ auto isNotType = compile([] (CCallHelpers& jit) {
+ emitFunctionPrologue(jit);
+ auto isNotType = jit.branchIfNotType(GPRInfo::argumentGPR0, JSType(FirstTypedArrayType), JSType(LastTypedArrayTypeExcludingDataView));
+ jit.move(CCallHelpers::TrustedImm32(false), GPRInfo::returnValueGPR);
+ auto done = jit.jump();
+ isNotType.link(&jit);
+ jit.move(CCallHelpers::TrustedImm32(true), GPRInfo::returnValueGPR);
+ done.link(&jit);
+ emitFunctionEpilogue(jit);
+ jit.ret();
+ });
+
+ CellLike cell;
+ for (unsigned i = JSC::FirstTypedArrayType; i <= JSC::LastTypedArrayTypeExcludingDataView; ++i) {
+ cell.type = JSType(i);
+ CHECK_EQ(invoke<bool>(isNotType, &cell), false);
+ }
+
+ cell.type = JSType(LastTypedArrayType);
+ CHECK_EQ(invoke<bool>(isNotType, &cell), true);
+ cell.type = JSType(FirstTypedArrayType - 1);
+ CHECK_EQ(invoke<bool>(isNotType, &cell), true);
+}
+
#define RUN(test) do { \
if (!shouldRun(#test)) \
break; \
@@ -2283,6 +2351,9 @@
RUN(testCagePreservesPACFailureBit());
+ RUN(testBranchIfType());
+ RUN(testBranchIfNotType());
+
RUN(testOrImmMem());
if (tasks.isEmpty())
Modified: trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp (261711 => 261712)
--- trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp 2020-05-14 21:50:41 UTC (rev 261711)
+++ trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp 2020-05-14 22:01:50 UTC (rev 261712)
@@ -185,6 +185,8 @@
static bool canBecomeGetArrayLength(Graph& graph, Node* node)
{
+ if (node->op() == GetArrayLength)
+ return true;
if (node->op() != GetById)
return false;
auto uid = node->cacheableIdentifier().uid();
Modified: trunk/Source/_javascript_Core/dfg/DFGArrayMode.h (261711 => 261712)
--- trunk/Source/_javascript_Core/dfg/DFGArrayMode.h 2020-05-14 21:50:41 UTC (rev 261711)
+++ trunk/Source/_javascript_Core/dfg/DFGArrayMode.h 2020-05-14 22:01:50 UTC (rev 261712)
@@ -237,6 +237,7 @@
return ArrayMode(type, arrayClass(), speculation(), conversion, action());
}
+ static constexpr SpeculatedType unusedIndexSpeculatedType = SpecInt32Only;
ArrayMode refine(Graph&, Node*, SpeculatedType base, SpeculatedType index, SpeculatedType value = SpecNone) const;
bool alreadyChecked(Graph&, Node*, const AbstractValue&) const;
Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (261711 => 261712)
--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2020-05-14 21:50:41 UTC (rev 261711)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2020-05-14 22:01:50 UTC (rev 261712)
@@ -1353,6 +1353,11 @@
def(HeapLocation(ArrayLengthLoc, MiscFields, node->child1()), LazyNode(node));
return;
+ case Array::ForceExit: {
+ write(SideState);
+ return;
+ }
+
default:
ASSERT(mode.isSomeTypedArrayView());
read(MiscFields);
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (261711 => 261712)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-05-14 21:50:41 UTC (rev 261711)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-05-14 22:01:50 UTC (rev 261712)
@@ -2021,6 +2021,14 @@
}
case GetArrayLength: {
+ ArrayMode arrayMode = node->arrayMode().refine(m_graph, node, node->child1()->prediction(), ArrayMode::unusedIndexSpeculatedType);
+ // We don't know how to handle generic and we only emit this in the Parser when we have checked the value is an Array/TypedArray.
+ if (arrayMode.type() == Array::Generic)
+ arrayMode = arrayMode.withType(Array::ForceExit);
+ ASSERT(arrayMode.isSpecific() || arrayMode.type() == Array::ForceExit);
+ node->setArrayMode(arrayMode);
+ blessArrayOperation(node->child1(), Edge(), node->child2(), lengthNeedsStorage);
+
fixEdge<KnownCellUse>(node->child1());
break;
}
@@ -3519,7 +3527,7 @@
}
}
- void blessArrayOperation(Edge base, Edge index, Edge& storageChild)
+ void blessArrayOperation(Edge base, Edge index, Edge& storageChild, bool (*storageCheck)(const ArrayMode&) = canCSEStorage)
{
Node* node = m_currentNode;
@@ -3539,7 +3547,7 @@
return;
default: {
- Node* storage = checkArray(node->arrayMode(), node->origin, base.node(), index.node());
+ Node* storage = checkArray(node->arrayMode(), node->origin, base.node(), index.node(), storageCheck);
if (!storage)
return;
@@ -3828,7 +3836,7 @@
}
arrayMode = arrayMode.refine(
- m_graph, node, node->child1()->prediction(), node->prediction());
+ m_graph, node, node->child1()->prediction(), ArrayMode::unusedIndexSpeculatedType);
if (arrayMode.type() == Array::Generic) {
// Check if the input is something that we can't get array length for, but for which we
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (261711 => 261712)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2020-05-14 21:50:41 UTC (rev 261711)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2020-05-14 22:01:50 UTC (rev 261712)
@@ -832,13 +832,14 @@
void SpeculativeJIT::checkArray(Node* node)
{
- ASSERT(node->arrayMode().isSpecific());
- ASSERT(!node->arrayMode().doesConversion());
+ ArrayMode arrayMode = node->arrayMode();
+ ASSERT(arrayMode.isSpecific());
+ ASSERT(!arrayMode.doesConversion());
SpeculateCellOperand base(this, node->child1());
GPRReg baseReg = base.gpr();
- if (node->arrayMode().alreadyChecked(m_jit.graph(), node, m_state.forNode(node->child1()))) {
+ if (arrayMode.alreadyChecked(m_jit.graph(), node, m_state.forNode(node->child1()))) {
// We can purge Empty check completely in this case of CheckArrayOrEmpty since CellUse only accepts SpecCell | SpecEmpty.
ASSERT(typeFilterFor(node->child1().useKind()) & SpecEmpty);
noResult(m_currentNode);
@@ -847,7 +848,7 @@
Optional<GPRTemporary> temp;
Optional<GPRReg> tempGPR;
- switch (node->arrayMode().type()) {
+ switch (arrayMode.type()) {
case Array::Int32:
case Array::Double:
case Array::Contiguous:
@@ -871,8 +872,7 @@
}
#endif
- switch (node->arrayMode().type()) {
- case Array::AnyTypedArray:
+ switch (arrayMode.type()) {
case Array::String:
RELEASE_ASSERT_NOT_REACHED(); // Should have been a Phantom(String:)
return;
@@ -885,7 +885,7 @@
m_jit.load8(MacroAssembler::Address(baseReg, JSCell::indexingTypeAndMiscOffset()), tempGPR.value());
speculationCheck(
BadIndexingType, JSValueSource::unboxedCell(baseReg), nullptr,
- jumpSlowForUnwantedArrayMode(tempGPR.value(), node->arrayMode()));
+ jumpSlowForUnwantedArrayMode(tempGPR.value(), arrayMode));
break;
}
case Array::DirectArguments:
@@ -894,12 +894,16 @@
case Array::ScopedArguments:
speculateCellTypeWithoutTypeFiltering(node->child1(), baseReg, ScopedArgumentsType);
break;
- default:
- speculateCellTypeWithoutTypeFiltering(
- node->child1(), baseReg,
- typeForTypedArrayType(node->arrayMode().typedArrayType()));
+ default: {
+ DFG_ASSERT(m_graph, node, arrayMode.isSomeTypedArrayView());
+
+ if (arrayMode.type() == Array::AnyTypedArray)
+ speculationCheck(BadType, JSValueSource::unboxedCell(baseReg), nullptr, m_jit.branchIfNotType(baseReg, JSType(FirstTypedArrayType), JSType(LastTypedArrayTypeExcludingDataView)));
+ else
+ speculateCellTypeWithoutTypeFiltering(node->child1(), baseReg, typeForTypedArrayType(arrayMode.typedArrayType()));
break;
}
+ }
if (isEmpty.isSet())
isEmpty.link(&m_jit);
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (261711 => 261712)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-05-14 21:50:41 UTC (rev 261711)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-05-14 22:01:50 UTC (rev 261712)
@@ -17959,11 +17959,15 @@
m_out.load8ZeroExt32(cell, m_heaps.JSCell_typeInfoType),
m_out.constInt32(ScopedArgumentsType));
- default:
+ default: {
+ DFG_ASSERT(m_graph, m_node, arrayMode.isSomeTypedArrayView());
+ if (arrayMode.type() == Array::AnyTypedArray)
+ return isTypedArrayView(cell);
return m_out.equal(
m_out.load8ZeroExt32(cell, m_heaps.JSCell_typeInfoType),
m_out.constInt32(typeForTypedArrayType(arrayMode.typedArrayType())));
}
+ }
}
LValue isFunction(LValue cell, SpeculatedType type = SpecFullTop)
Modified: trunk/Source/_javascript_Core/jit/AssemblyHelpers.h (261711 => 261712)
--- trunk/Source/_javascript_Core/jit/AssemblyHelpers.h 2020-05-14 21:50:41 UTC (rev 261711)
+++ trunk/Source/_javascript_Core/jit/AssemblyHelpers.h 2020-05-14 22:01:50 UTC (rev 261712)
@@ -980,14 +980,31 @@
Below, Address(cellGPR, JSCell::typeInfoTypeOffset()), TrustedImm32(ObjectType));
}
- Jump branchIfType(GPRReg cellGPR, JSType type)
+ // Note that first and last are inclusive.
+ Jump branchIfType(GPRReg cellGPR, JSType first, Optional<JSType> last = WTF::nullopt)
{
- return branch8(Equal, Address(cellGPR, JSCell::typeInfoTypeOffset()), TrustedImm32(type));
+ if (last && *last != first) {
+ ASSERT(*last > first);
+ GPRReg scratch = scratchRegister();
+ load8(Address(cellGPR, JSCell::typeInfoTypeOffset()), scratch);
+ sub32(TrustedImm32(first), scratch);
+ return branch32(BelowOrEqual, scratch, TrustedImm32(*last - first));
+ }
+
+ return branch8(Equal, Address(cellGPR, JSCell::typeInfoTypeOffset()), TrustedImm32(first));
}
-
- Jump branchIfNotType(GPRReg cellGPR, JSType type)
+
+ Jump branchIfNotType(GPRReg cellGPR, JSType first, Optional<JSType> last = WTF::nullopt)
{
- return branch8(NotEqual, Address(cellGPR, JSCell::typeInfoTypeOffset()), TrustedImm32(type));
+ if (last && *last != first) {
+ ASSERT(*last > first);
+ GPRReg scratch = scratchRegister();
+ load8(Address(cellGPR, JSCell::typeInfoTypeOffset()), scratch);
+ sub32(TrustedImm32(first), scratch);
+ return branch32(Above, scratch, TrustedImm32(*last - first));
+ }
+
+ return branch8(NotEqual, Address(cellGPR, JSCell::typeInfoTypeOffset()), TrustedImm32(first));
}
// FIXME: rename these to make it clear that they require their input to be a cell.
Modified: trunk/Source/_javascript_Core/runtime/JSType.h (261711 => 261712)
--- trunk/Source/_javascript_Core/runtime/JSType.h 2020-05-14 21:50:41 UTC (rev 261711)
+++ trunk/Source/_javascript_Core/runtime/JSType.h 2020-05-14 22:01:50 UTC (rev 261712)
@@ -132,6 +132,7 @@
static constexpr uint32_t FirstTypedArrayType = Int8ArrayType;
static constexpr uint32_t LastTypedArrayType = DataViewType;
+static constexpr uint32_t LastTypedArrayTypeExcludingDataView = LastTypedArrayType - 1;
// LastObjectType should be MaxJSType (not LastJSCObjectType) since embedders can add their extended object types after the enums listed in JSType.
static constexpr uint32_t FirstObjectType = ObjectType;
