Diff
Added: trunk/JSTests/stress/for-of-get-by-val-marks-clobbers-exit-state.js (0 => 261313)
--- trunk/JSTests/stress/for-of-get-by-val-marks-clobbers-exit-state.js (rev 0)
+++ trunk/JSTests/stress/for-of-get-by-val-marks-clobbers-exit-state.js 2020-05-07 18:00:12 UTC (rev 261313)
@@ -0,0 +1,10 @@
+const a0 = new Array(100);
+a0.x = 0;
+
+function foo() {
+ for (const q of a0) {}
+}
+
+for (let i = 0; i < 1000; i++) {
+ foo();
+}
Added: trunk/JSTests/stress/put-by-val-correctly-clobbers-exit-state-when-misprofiling-index.js (0 => 261313)
--- trunk/JSTests/stress/put-by-val-correctly-clobbers-exit-state-when-misprofiling-index.js (rev 0)
+++ trunk/JSTests/stress/put-by-val-correctly-clobbers-exit-state-when-misprofiling-index.js 2020-05-07 18:00:12 UTC (rev 261313)
@@ -0,0 +1,10 @@
+let foo = $vm.createBuiltin(`(function (array, index) {
+ index = @idWithProfile(index, "SpecObject");
+ return array[index];
+})`);
+noInline(foo);
+
+for (let i = 0; i < 1e5; ++i) {
+ if (foo([1,2], 0) !== 1)
+ throw new Error();
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (261312 => 261313)
--- trunk/Source/_javascript_Core/ChangeLog 2020-05-07 17:58:19 UTC (rev 261312)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-05-07 18:00:12 UTC (rev 261313)
@@ -1,3 +1,23 @@
+2020-05-07 Keith Miller <keith_mil...@apple.com>
+
+ Fix ArrayMode nodes after r261260
+ https://bugs.webkit.org/show_bug.cgi?id=211543
+
+ Reviewed by Yusuke Suzuki.
+
+ I accidentally ran tests with a release build rather than
+ release+assert when uploading r261260. This patch skips the
+ CheckArray node in the ArrayMode clobbersTop() logic before
+ Fixup. And also marks a GetArrayLength in the TypedArray
+ intrsinics as ExitOK.
+
+ This patch also relands r261260.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::handleIntrinsicGetter):
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+
2020-05-07 Ryan Haddad <ryanhad...@apple.com>
Unreviewed, reverting r261260.
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (261312 => 261313)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2020-05-07 17:58:19 UTC (rev 261312)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2020-05-07 18:00:12 UTC (rev 261313)
@@ -3750,7 +3750,11 @@
});
Node* lengthNode = addToGraph(GetArrayLength, OpInfo(ArrayMode(arrayType, Array::Read).asWord()), thisNode);
+ // Our ArrayMode shouldn't cause us to exit here so we should be ok to exit without effects.
+ m_exitOK = true;
+ addToGraph(ExitOK);
+
if (!logSize) {
set(result, lengthNode);
return true;
@@ -6901,6 +6905,9 @@
Node* iterable = get(bytecode.m_iterable);
Node* butterfly = addToGraph(GetButterfly, iterable);
Node* length = addToGraph(GetArrayLength, OpInfo(arrayMode.asWord()), iterable, butterfly);
+ // GetArrayLength is pessimized prior to fixup.
+ m_exitOK = true;
+ addToGraph(ExitOK);
Node* isOutOfBounds = addToGraph(CompareGreaterEq, Edge(index, Int32Use), Edge(length, Int32Use));
isDone = addToGraph(ArithBitOr, isDone, isOutOfBounds);
Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (261312 => 261313)
--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2020-05-07 17:58:19 UTC (rev 261312)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2020-05-07 18:00:12 UTC (rev 261313)
@@ -155,6 +155,46 @@
read(World);
write(Heap);
};
+
+ // Since Fixup can widen our ArrayModes based on profiling from other nodes we pessimistically assume
+ // all nodes with an ArrayMode can clobber top. We allow some nodes like CheckArray because they can
+ // only exit.
+ if (graph.m_planStage < PlanStage::AfterFixup && node->hasArrayMode()) {
+ switch (node->op()) {
+ case CheckArray:
+ case CheckArrayOrEmpty:
+ break;
+ case GetIndexedPropertyStorage:
+ case GetArrayLength:
+ case GetVectorLength:
+ case InByVal:
+ case PutByValDirect:
+ case PutByVal:
+ case PutByValAlias:
+ case GetByVal:
+ case StringCharAt:
+ case StringCharCodeAt:
+ case StringCodePointAt:
+ case Arrayify:
+ case ArrayifyToStructure:
+ case ArrayPush:
+ case ArrayPop:
+ case ArrayIndexOf:
+ case HasIndexedProperty:
+ case AtomicsAdd:
+ case AtomicsAnd:
+ case AtomicsCompareExchange:
+ case AtomicsExchange:
+ case AtomicsLoad:
+ case AtomicsOr:
+ case AtomicsStore:
+ case AtomicsSub:
+ case AtomicsXor:
+ return clobberTop();
+ default:
+ DFG_CRASH(graph, node, "Unhandled ArrayMode opcode.");
+ }
+ }
switch (node->op()) {
case JSConstant:
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (261312 => 261313)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-05-07 17:58:19 UTC (rev 261312)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-05-07 18:00:12 UTC (rev 261313)
@@ -180,6 +180,10 @@
{
NodeType op = node->op();
+#if ASSERT_ENABLED
+ bool usedToClobberExitState = clobbersExitState(m_graph, node);
+#endif
+
switch (op) {
case SetLocal: {
// This gets handled by fixupGetAndSetLocalsInBlock().
@@ -1281,7 +1285,7 @@
node->setArrayMode(ArrayMode(Array::Generic, node->arrayMode().action()));
break;
}
-
+
node->setArrayMode(
node->arrayMode().refine(
m_graph, node, base->prediction(), index->prediction()));
@@ -2720,6 +2724,12 @@
break;
#endif // not ASSERT_ENABLED
}
+
+#if ASSERT_ENABLED
+ // It would be invalid for Fixup to take a node that didn't clobber exit state and mark it as clobbering afterwords.
+ DFG_ASSERT(m_graph, node, usedToClobberExitState || !clobbersExitState(m_graph, node));
+#endif
+
}
void watchHavingABadTime(Node* node)
