- Revision
- 240376
- Author
- [email protected]
- Date
- 2019-01-23 17:21:18 -0800 (Wed, 23 Jan 2019)
Log Message
Cherry-pick r240024. rdar://problem/47458299
[JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)
https://bugs.webkit.org/show_bug.cgi?id=193438
<rdar://problem/45581249>
Reviewed by Saam Barati and Keith Miller.
JSTests:
Under the heavy load (like, compiling WebKit), AI in this code can broaden type information after the 1st run.
Then, GetByVal(String) crashed.
* stress/string-get-by-val-lowering.js: Added.
(shouldBe):
(test):
* stress/type-for-get-by-val-can-be-widen-after-ai.js: Added.
(Hello):
(foo):
Source/_javascript_Core:
GetByVal(Array::String) emits Check(String) before that. But AI can broaden type constraint in the second run.
After the first run removes Check(String), it would happen that AI starts saying the type of 1st child is not String.
To claim that it *is* a String type, we should use KnownStringUse here.
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode): StringCharAt and GetByVal(Array::String) share the underlying compiler code. We should
change StringUse => KnownStringUse for StringCharAt too. And StringCharAt and StringCharCodeAt potentially have the same
problem. This patch fixes it too.
* dfg/DFGSSALoweringPhase.cpp:
(JSC::DFG::SSALoweringPhase::lowerBoundsCheck):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
(JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240024 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-607-branch/JSTests/ChangeLog (240375 => 240376)
--- branches/safari-607-branch/JSTests/ChangeLog 2019-01-24 01:21:14 UTC (rev 240375)
+++ branches/safari-607-branch/JSTests/ChangeLog 2019-01-24 01:21:18 UTC (rev 240376)
@@ -1,5 +1,63 @@
2019-01-23 Alan Coon <[email protected]>
+ Cherry-pick r240024. rdar://problem/47458299
+
+ [JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)
+ https://bugs.webkit.org/show_bug.cgi?id=193438
+ <rdar://problem/45581249>
+
+ Reviewed by Saam Barati and Keith Miller.
+
+ JSTests:
+
+ Under the heavy load (like, compiling WebKit), AI in this code can broaden type information after the 1st run.
+ Then, GetByVal(String) crashed.
+
+ * stress/string-get-by-val-lowering.js: Added.
+ (shouldBe):
+ (test):
+ * stress/type-for-get-by-val-can-be-widen-after-ai.js: Added.
+ (Hello):
+ (foo):
+
+ Source/_javascript_Core:
+
+ GetByVal(Array::String) emits Check(String) before that. But AI can broaden type constraint in the second run.
+ After the first run removes Check(String), it would happen that AI starts saying the type of 1st child is not String.
+ To claim that it *is* a String type, we should use KnownStringUse here.
+
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode): StringCharAt and GetByVal(Array::String) share the underlying compiler code. We should
+ change StringUse => KnownStringUse for StringCharAt too. And StringCharAt and StringCharCodeAt potentially have the same
+ problem. This patch fixes it too.
+ * dfg/DFGSSALoweringPhase.cpp:
+ (JSC::DFG::SSALoweringPhase::lowerBoundsCheck):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
+ (JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240024 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-01-15 Yusuke Suzuki <[email protected]>
+
+ [JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)
+ https://bugs.webkit.org/show_bug.cgi?id=193438
+ <rdar://problem/45581249>
+
+ Reviewed by Saam Barati and Keith Miller.
+
+ Under the heavy load (like, compiling WebKit), AI in this code can broaden type information after the 1st run.
+ Then, GetByVal(String) crashed.
+
+ * stress/string-get-by-val-lowering.js: Added.
+ (shouldBe):
+ (test):
+ * stress/type-for-get-by-val-can-be-widen-after-ai.js: Added.
+ (Hello):
+ (foo):
+
+2019-01-23 Alan Coon <[email protected]>
+
Cherry-pick r239961. rdar://problem/47458424
[BigInt] Literal parsing is crashing when used inside a Object Literal
Added: branches/safari-607-branch/JSTests/stress/string-get-by-val-lowering.js (0 => 240376)
--- branches/safari-607-branch/JSTests/stress/string-get-by-val-lowering.js (rev 0)
+++ branches/safari-607-branch/JSTests/stress/string-get-by-val-lowering.js 2019-01-24 01:21:18 UTC (rev 240376)
@@ -0,0 +1,17 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+noInline(shouldBe);
+
+function test(value)
+{
+ return value[2];
+}
+noInline(test);
+
+for (var i = 0; i < 1e4; ++i) {
+ shouldBe(test("Hello"), 'l');
+ shouldBe(test("World"), 'r');
+ shouldBe(test("Nice"), 'c');
+}
Added: branches/safari-607-branch/JSTests/stress/type-for-get-by-val-can-be-widen-after-ai.js (0 => 240376)
--- branches/safari-607-branch/JSTests/stress/type-for-get-by-val-can-be-widen-after-ai.js (rev 0)
+++ branches/safari-607-branch/JSTests/stress/type-for-get-by-val-can-be-widen-after-ai.js 2019-01-24 01:21:18 UTC (rev 240376)
@@ -0,0 +1,25 @@
+//@ runDefault("--jitPolicyScale=0")
+// Run with for i in {1..1000}; do echo $i && VM=/path/to/WebKit/WebKitBuild/Debug/ && DYLD_FRAMEWORK_PATH=$VM $VM/jsc --useDollarVM=1 --jitPolicyScale=0 type-for-get-by-val-can-be-widen-after-ai.js ; done
+
+function Hello(y) {
+ this.y = y;
+ this.x = foo(this.y);
+}
+function foo(z) {
+ try {
+ for (var i = 0; i < 1; i++) {
+ z[i];
+ }
+ } catch {
+ }
+}
+new Hello('a');
+new Hello('a');
+for (let i = 0; i < 100; ++i) {
+ new Hello();
+}
+
+// Busy loop to let the crash reporter have a chance to capture the crash log for the Compiler thread.
+for (let i = 0; i < 1000000; ++i) {
+ $vm.ftlTrue();
+}
Modified: branches/safari-607-branch/Source/_javascript_Core/ChangeLog (240375 => 240376)
--- branches/safari-607-branch/Source/_javascript_Core/ChangeLog 2019-01-24 01:21:14 UTC (rev 240375)
+++ branches/safari-607-branch/Source/_javascript_Core/ChangeLog 2019-01-24 01:21:18 UTC (rev 240376)
@@ -1,5 +1,67 @@
2019-01-23 Alan Coon <[email protected]>
+ Cherry-pick r240024. rdar://problem/47458299
+
+ [JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)
+ https://bugs.webkit.org/show_bug.cgi?id=193438
+ <rdar://problem/45581249>
+
+ Reviewed by Saam Barati and Keith Miller.
+
+ JSTests:
+
+ Under the heavy load (like, compiling WebKit), AI in this code can broaden type information after the 1st run.
+ Then, GetByVal(String) crashed.
+
+ * stress/string-get-by-val-lowering.js: Added.
+ (shouldBe):
+ (test):
+ * stress/type-for-get-by-val-can-be-widen-after-ai.js: Added.
+ (Hello):
+ (foo):
+
+ Source/_javascript_Core:
+
+ GetByVal(Array::String) emits Check(String) before that. But AI can broaden type constraint in the second run.
+ After the first run removes Check(String), it would happen that AI starts saying the type of 1st child is not String.
+ To claim that it *is* a String type, we should use KnownStringUse here.
+
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode): StringCharAt and GetByVal(Array::String) share the underlying compiler code. We should
+ change StringUse => KnownStringUse for StringCharAt too. And StringCharAt and StringCharCodeAt potentially have the same
+ problem. This patch fixes it too.
+ * dfg/DFGSSALoweringPhase.cpp:
+ (JSC::DFG::SSALoweringPhase::lowerBoundsCheck):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
+ (JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240024 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-01-15 Yusuke Suzuki <[email protected]>
+
+ [JSC] Use KnownStringUse for GetByVal(Array::String) since AI would offer wider type information and offer non-string type after removing Check(String)
+ https://bugs.webkit.org/show_bug.cgi?id=193438
+ <rdar://problem/45581249>
+
+ Reviewed by Saam Barati and Keith Miller.
+
+ GetByVal(Array::String) emits Check(String) before that. But AI can broaden type constraint in the second run.
+ After the first run removes Check(String), it would happen that AI starts saying the type of 1st child is not String.
+ To claim that it *is* a String type, we should use KnownStringUse here.
+
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode): StringCharAt and GetByVal(Array::String) share the underlying compiler code. We should
+ change StringUse => KnownStringUse for StringCharAt too. And StringCharAt and StringCharCodeAt potentially have the same
+ problem. This patch fixes it too.
+ * dfg/DFGSSALoweringPhase.cpp:
+ (JSC::DFG::SSALoweringPhase::lowerBoundsCheck):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
+ (JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):
+
+2019-01-23 Alan Coon <[email protected]>
+
Cherry-pick r240023. rdar://problem/47458203
Try ripping out inferred types because it might be a performance improvement
Modified: branches/safari-607-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (240375 => 240376)
--- branches/safari-607-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2019-01-24 01:21:14 UTC (rev 240375)
+++ branches/safari-607-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2019-01-24 01:21:18 UTC (rev 240376)
@@ -777,7 +777,7 @@
// Currently we have no good way of refining these.
ASSERT(node->arrayMode() == ArrayMode(Array::String, Array::Read));
blessArrayOperation(node->child1(), node->child2(), node->child3());
- fixEdge<KnownCellUse>(node->child1());
+ fixEdge<KnownStringUse>(node->child1());
fixEdge<Int32Use>(node->child2());
break;
}
@@ -895,6 +895,10 @@
break;
case Array::ForceExit:
break;
+ case Array::String:
+ fixEdge<KnownStringUse>(m_graph.varArgChild(node, 0));
+ fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
+ break;
default:
fixEdge<KnownCellUse>(m_graph.varArgChild(node, 0));
fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
Modified: branches/safari-607-branch/Source/_javascript_Core/dfg/DFGSSALoweringPhase.cpp (240375 => 240376)
--- branches/safari-607-branch/Source/_javascript_Core/dfg/DFGSSALoweringPhase.cpp 2019-01-24 01:21:14 UTC (rev 240375)
+++ branches/safari-607-branch/Source/_javascript_Core/dfg/DFGSSALoweringPhase.cpp 2019-01-24 01:21:18 UTC (rev 240376)
@@ -124,6 +124,10 @@
case Array::SlowPutArrayStorage:
op = GetVectorLength;
break;
+ case Array::String:
+ // When we need to support this, it will require additional code since base's useKind is KnownStringUse.
+ DFG_CRASH(m_graph, m_node, "Array::String's base.useKind() is KnownStringUse");
+ break;
default:
break;
}
@@ -130,7 +134,7 @@
Node* length = m_insertionSet.insertNode(
m_nodeIndex, SpecInt32Only, op, m_node->origin,
- OpInfo(m_node->arrayMode().asWord()), base, storage);
+ OpInfo(m_node->arrayMode().asWord()), Edge(base.node(), KnownCellUse), storage);
m_insertionSet.insertNode(
m_nodeIndex, SpecInt32Only, CheckInBounds, m_node->origin,
index, Edge(length, KnownInt32Use));
Modified: branches/safari-607-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (240375 => 240376)
--- branches/safari-607-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2019-01-24 01:21:14 UTC (rev 240375)
+++ branches/safari-607-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2019-01-24 01:21:18 UTC (rev 240376)
@@ -6587,7 +6587,7 @@
void compileStringCharAt()
{
- LValue base = lowCell(m_graph.child(m_node, 0));
+ LValue base = lowString(m_graph.child(m_node, 0));
LValue index = lowInt32(m_graph.child(m_node, 1));
LValue storage = lowStorage(m_graph.child(m_node, 2));
@@ -6701,7 +6701,7 @@
LBasicBlock is16Bit = m_out.newBlock();
LBasicBlock continuation = m_out.newBlock();
- LValue base = lowCell(m_node->child1());
+ LValue base = lowString(m_node->child1());
LValue index = lowInt32(m_node->child2());
LValue storage = lowStorage(m_node->child3());
