Title: [261260] trunk
- Revision
- 261260
- Author
- keith_mil...@apple.com
- Date
- 2020-05-06 17:00:33 -0700 (Wed, 06 May 2020)
Log Message
DFG ByVal nodes with ArrayModes should clobberTop until Fixup phase runs.
https://bugs.webkit.org/show_bug.cgi?id=211531
Reviewed by Yusuke Suzuki.
JSTests:
* stress/for-of-get-by-val-marks-clobbers-exit-state.js: Added.
(foo):
* stress/put-by-val-correctly-clobbers-exit-state-when-misprofiling-index.js: Added.
(let.foo.vm.createBuiltin):
Source/_javascript_Core:
When parsing bytecode we may pick a relatively constrained
ArrayMode based on our profiling. Some of these modes may not
clobber exit state. However, Fixup sometimes wants to widen this
to a more generic mode based on other data. This causes us to
think it was valid to exit immediately after the
GetByVal/HasIndexedProperty, which would be wrong with the wider
ArrayMode. We may also incorrectly insert invalidition points
if clobberize gives us the wrong data.
To fix this clobberize should say All ByVal nodes clobberTop()
until after fixup. Additionally, this patch adds an assertion that
nodes don't go from not clobbering exit state to clobbering exit
state during fixup.
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::performFixup):
* dfg/DFGGraph.h:
Modified Paths
Diff
Modified: trunk/JSTests/ChangeLog (261259 => 261260)
--- trunk/JSTests/ChangeLog 2020-05-06 23:50:19 UTC (rev 261259)
+++ trunk/JSTests/ChangeLog 2020-05-07 00:00:33 UTC (rev 261260)
@@ -1,3 +1,15 @@
+2020-05-06 Keith Miller <keith_mil...@apple.com>
+
+ DFG ByVal nodes with ArrayModes should clobberTop until Fixup phase runs.
+ https://bugs.webkit.org/show_bug.cgi?id=211531
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/for-of-get-by-val-marks-clobbers-exit-state.js: Added.
+ (foo):
+ * stress/put-by-val-correctly-clobbers-exit-state-when-misprofiling-index.js: Added.
+ (let.foo.vm.createBuiltin):
+
2020-05-05 Ross Kirsling <ross.kirsl...@sony.com>
[ECMA-402] Implement Intl.Locale
Modified: trunk/Source/_javascript_Core/ChangeLog (261259 => 261260)
--- trunk/Source/_javascript_Core/ChangeLog 2020-05-06 23:50:19 UTC (rev 261259)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-05-07 00:00:33 UTC (rev 261260)
@@ -1,3 +1,31 @@
+2020-05-06 Keith Miller <keith_mil...@apple.com>
+
+ DFG ByVal nodes with ArrayModes should clobberTop until Fixup phase runs.
+ https://bugs.webkit.org/show_bug.cgi?id=211531
+
+ Reviewed by Yusuke Suzuki.
+
+ When parsing bytecode we may pick a relatively constrained
+ ArrayMode based on our profiling. Some of these modes may not
+ clobber exit state. However, Fixup sometimes wants to widen this
+ to a more generic mode based on other data. This causes us to
+ think it was valid to exit immediately after the
+ GetByVal/HasIndexedProperty, which would be wrong with the wider
+ ArrayMode. We may also incorrectly insert invalidition points
+ if clobberize gives us the wrong data.
+
+ To fix this clobberize should say All ByVal nodes clobberTop()
+ until after fixup. Additionally, this patch adds an assertion that
+ nodes don't go from not clobbering exit state to clobbering exit
+ state during fixup.
+
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ (JSC::DFG::performFixup):
+ * dfg/DFGGraph.h:
+
2020-05-06 Darin Adler <da...@apple.com>
Make a helper for the pattern of ICU functions that may need to be called twice to populate a buffer
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (261259 => 261260)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2020-05-06 23:50:19 UTC (rev 261259)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2020-05-07 00:00:33 UTC (rev 261260)
@@ -6901,6 +6901,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 (261259 => 261260)
--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2020-05-06 23:50:19 UTC (rev 261259)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2020-05-07 00:00:33 UTC (rev 261260)
@@ -155,6 +155,11 @@
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.
+ if (graph.m_planStage < PlanStage::AfterFixup && node->hasArrayMode())
+ return clobberTop();
switch (node->op()) {
case JSConstant:
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (261259 => 261260)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-05-06 23:50:19 UTC (rev 261259)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-05-07 00:00:33 UTC (rev 261260)
@@ -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)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
