Modified: trunk/JSTests/ChangeLog (231192 => 231193)
--- trunk/JSTests/ChangeLog 2018-05-01 02:22:02 UTC (rev 231192)
+++ trunk/JSTests/ChangeLog 2018-05-01 06:04:33 UTC (rev 231193)
@@ -1,3 +1,13 @@
+2018-04-30 Saam Barati <[email protected]>
+
+ ToString constant folds without preserving checks, causing us to break assumptions that the code would OSR exit
+ https://bugs.webkit.org/show_bug.cgi?id=185149
+ <rdar://problem/39455917>
+
+ Reviewed by Filip Pizlo.
+
+ * stress/keep-checks-when-converting-to-lazy-js-constant-in-strength-reduction.js: Added.
+
2018-04-29 Filip Pizlo <[email protected]>
LICM shouldn't hoist nodes if hoisted nodes exited in that code block
Added: trunk/JSTests/stress/keep-checks-when-converting-to-lazy-js-constant-in-strength-reduction.js (0 => 231193)
--- trunk/JSTests/stress/keep-checks-when-converting-to-lazy-js-constant-in-strength-reduction.js (rev 0)
+++ trunk/JSTests/stress/keep-checks-when-converting-to-lazy-js-constant-in-strength-reduction.js 2018-05-01 06:04:33 UTC (rev 231193)
@@ -0,0 +1,7 @@
+//@ runDefault("--jitPolicyScale=0", "--useConcurrentJIT=false")
+
+let bar;
+for (let i = 0; i < 20; ++i) {
+ bar = i ** 0;
+ bar + '';
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (231192 => 231193)
--- trunk/Source/_javascript_Core/ChangeLog 2018-05-01 02:22:02 UTC (rev 231192)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-05-01 06:04:33 UTC (rev 231193)
@@ -1,3 +1,24 @@
+2018-04-30 Saam Barati <[email protected]>
+
+ ToString constant folds without preserving checks, causing us to break assumptions that the code would OSR exit
+ https://bugs.webkit.org/show_bug.cgi?id=185149
+ <rdar://problem/39455917>
+
+ Reviewed by Filip Pizlo.
+
+ The bug was that we were deleting checks that we shouldn't have deleted.
+ This patch makes a helper inside strength reduction that converts to
+ a LazyJSConstant while maintaining checks, and switches users of the
+ node API inside strength reduction to instead call the helper function.
+
+ This patch also fixes a potential bug where StringReplace and
+ StringReplaceRegExp may not preserve all their checks.
+
+
+ * dfg/DFGStrengthReductionPhase.cpp:
+ (JSC::DFG::StrengthReductionPhase::handleNode):
+ (JSC::DFG::StrengthReductionPhase::convertToLazyJSValue):
+
2018-04-29 Filip Pizlo <[email protected]>
LICM shouldn't hoist nodes if hoisted nodes exited in that code block
Modified: trunk/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp (231192 => 231193)
--- trunk/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp 2018-05-01 02:22:02 UTC (rev 231192)
+++ trunk/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp 2018-05-01 06:04:33 UTC (rev 231193)
@@ -361,8 +361,7 @@
StringBuilder builder;
builder.append(leftString);
builder.append(rightString);
- m_node->convertToLazyJSConstant(
- m_graph, LazyJSValue::newString(m_graph, builder.toString()));
+ convertToLazyJSValue(m_node, LazyJSValue::newString(m_graph, builder.toString()));
m_changed = true;
}
break;
@@ -389,8 +388,7 @@
if (!!extraString)
builder.append(extraString);
- m_node->convertToLazyJSConstant(
- m_graph, LazyJSValue::newString(m_graph, builder.toString()));
+ convertToLazyJSValue(m_node, LazyJSValue::newString(m_graph, builder.toString()));
m_changed = true;
break;
}
@@ -412,7 +410,7 @@
result = String::numberToStringECMAScript(value.asNumber());
if (!result.isNull()) {
- m_node->convertToLazyJSConstant(m_graph, LazyJSValue::newString(m_graph, result));
+ convertToLazyJSValue(m_node, LazyJSValue::newString(m_graph, result));
m_changed = true;
}
}
@@ -432,7 +430,7 @@
JSValue value = child1->constant()->value();
if (value && value.isNumber()) {
String result = toStringWithRadix(value.asNumber(), m_node->validRadixConstant());
- m_node->convertToLazyJSConstant(m_graph, LazyJSValue::newString(m_graph, result));
+ convertToLazyJSValue(m_node, LazyJSValue::newString(m_graph, result));
m_changed = true;
}
}
@@ -866,6 +864,10 @@
NodeOrigin origin = m_node->origin;
+ // Preserve any checks we have.
+ m_insertionSet.insertNode(
+ m_nodeIndex, SpecNone, Check, origin, m_node->children.justChecks());
+
if (regExp->global()) {
m_insertionSet.insertNode(
m_nodeIndex, SpecNone, SetRegExpObjectLastIndex, origin,
@@ -883,8 +885,7 @@
if (lastIndex < string.length())
builder.append(string, lastIndex, string.length() - lastIndex);
- m_node->convertToLazyJSConstant(
- m_graph, LazyJSValue::newString(m_graph, builder.toString()));
+ m_node->convertToLazyJSConstant(m_graph, LazyJSValue::newString(m_graph, builder.toString()));
}
m_node->origin = origin;
@@ -946,6 +947,12 @@
{
convertToIdentityOverChild(1);
}
+
+ void convertToLazyJSValue(Node* node, LazyJSValue value)
+ {
+ m_insertionSet.insertCheck(m_graph, m_nodeIndex, node);
+ node->convertToLazyJSConstant(m_graph, value);
+ }
void handleCommutativity()
{
