Diff
Modified: trunk/JSTests/ChangeLog (214027 => 214028)
--- trunk/JSTests/ChangeLog 2017-03-16 04:10:53 UTC (rev 214027)
+++ trunk/JSTests/ChangeLog 2017-03-16 04:49:47 UTC (rev 214028)
@@ -1,3 +1,16 @@
+2017-03-15 Yusuke Suzuki <utatane....@gmail.com>
+
+ [DFG] ToString operation should have fixup for primitives to say this node does not have side effects
+ https://bugs.webkit.org/show_bug.cgi?id=169544
+
+ Reviewed by Saam Barati.
+
+ * microbenchmarks/template-string-array.js: Added.
+ (test):
+ * stress/to-string-non-cell-use.js: Added.
+ (shouldBe):
+ (shouldThrow):
+
2017-03-13 Commit Queue <commit-qu...@webkit.org>
Unreviewed, rolling out r213856.
Added: trunk/JSTests/microbenchmarks/template-string-array.js (0 => 214028)
--- trunk/JSTests/microbenchmarks/template-string-array.js (rev 0)
+++ trunk/JSTests/microbenchmarks/template-string-array.js 2017-03-16 04:49:47 UTC (rev 214028)
@@ -0,0 +1,9 @@
+var array = [1, 2, 3];
+function test()
+{
+ return `${array[0]}, ${array[1]}, ${array[2]}, ${array[0]}, ${array[1]}, ${array[2]}`;
+}
+noInline(test);
+
+for (var i = 0; i < 1e5; ++i)
+ test();
Added: trunk/JSTests/stress/to-string-non-cell-use.js (0 => 214028)
--- trunk/JSTests/stress/to-string-non-cell-use.js (rev 0)
+++ trunk/JSTests/stress/to-string-non-cell-use.js 2017-03-16 04:49:47 UTC (rev 214028)
@@ -0,0 +1,43 @@
+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+function shouldThrow(func, errorMessage)
+{
+ var errorThrown = false;
+ var error = null;
+ try {
+ func();
+ } catch (e) {
+ errorThrown = true;
+ error = e;
+ }
+ if (!errorThrown)
+ throw new Error('not thrown');
+ if (String(error) !== errorMessage)
+ throw new Error(`bad error: ${String(error)}`);
+}
+
+function toString(value)
+{
+ return `${value}`;
+}
+noInline(toString);
+
+for (var i = 0; i < 1e4; ++i) {
+ shouldBe(toString(i), i + "");
+ shouldBe(toString(null), "null");
+ shouldBe(toString(undefined), "undefined");
+ shouldBe(toString(10.5), "10.5");
+ shouldBe(toString(-10.5), "-10.5");
+ shouldBe(toString(true), "true");
+ shouldBe(toString(false), "false");
+ shouldBe(toString(0 / 0), "NaN");
+}
+
+shouldBe(toString("HELLO"), "HELLO");
+shouldThrow(() => {
+ toString(Symbol("Cocoa"));
+}, `TypeError: Cannot convert a symbol to a string`);
Modified: trunk/Source/_javascript_Core/ChangeLog (214027 => 214028)
--- trunk/Source/_javascript_Core/ChangeLog 2017-03-16 04:10:53 UTC (rev 214027)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-03-16 04:49:47 UTC (rev 214028)
@@ -1,3 +1,47 @@
+2017-03-15 Yusuke Suzuki <utatane....@gmail.com>
+
+ [DFG] ToString operation should have fixup for primitives to say this node does not have side effects
+ https://bugs.webkit.org/show_bug.cgi?id=169544
+
+ Reviewed by Saam Barati.
+
+ Our DFG ToString only considers well about String operands. While ToString(non cell operand) does not have
+ any side effect, it is not modeled well in DFG.
+
+ This patch introduces a fixup for ToString with NonCellUse edge. If this edge is set, ToString does not
+ clobber things (like ToLowerCase, producing String). And ToString(NonCellUse) allows us to perform CSE!
+
+ Our microbenchmark shows 32.9% improvement due to dropped GetButterfly and CSE for ToString().
+
+ baseline patched
+
+ template-string-array 12.6284+-0.2766 ^ 9.4998+-0.2295 ^ definitely 1.3293x faster
+
+ And SixSpeed template_string.es6 shows 16.68x performance improvement due to LICM onto this non-side-effectful ToString().
+
+ baseline patched
+
+ template_string.es6 3229.7343+-40.5705 ^ 193.6077+-36.3349 ^ definitely 16.6818x faster
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupToStringOrCallStringConstructor):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOnCell):
+ (JSC::DFG::SpeculativeJIT::speculateNotCell):
+ * dfg/DFGSpeculativeJIT.h:
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileToStringOrCallStringConstructor):
+ (JSC::FTL::DFG::LowerDFGToB3::lowNotCell):
+ (JSC::FTL::DFG::LowerDFGToB3::speculateNotCell):
+
2017-03-15 Ryan Haddad <ryanhad...@apple.com>
Revert part of r213978 to see if it resolves LayoutTest crashes.
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (214027 => 214028)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2017-03-16 04:10:53 UTC (rev 214027)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2017-03-16 04:49:47 UTC (rev 214028)
@@ -1858,6 +1858,7 @@
m_graph.registerStructure(m_graph.globalObjectFor(node->origin.semantic)->stringObjectStructure()));
break;
case StringOrStringObjectUse:
+ case NotCellUse:
break;
case CellUse:
case UntypedUse:
Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (214027 => 214028)
--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2017-03-16 04:10:53 UTC (rev 214027)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2017-03-16 04:49:47 UTC (rev 214028)
@@ -1408,6 +1408,10 @@
read(World);
write(Heap);
return;
+
+ case NotCellUse:
+ def(PureValue(node));
+ return;
default:
RELEASE_ASSERT_NOT_REACHED();
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (214027 => 214028)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2017-03-16 04:10:53 UTC (rev 214027)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2017-03-16 04:49:47 UTC (rev 214028)
@@ -2235,6 +2235,16 @@
fixEdge<CellUse>(node->child1());
return;
}
+
+ // ToString(Symbol) throws an error. So if the child1 can include Symbols,
+ // we need to care about it in the clobberize. In the following case,
+ // since NotCellUse edge filter is used and this edge filters Symbols,
+ // we can say that ToString never throws an error!
+ if (node->child1()->shouldSpeculateNotCell()) {
+ fixEdge<NotCellUse>(node->child1());
+ node->clearFlags(NodeMustGenerate);
+ return;
+ }
}
bool attemptToMakeFastStringAdd(Node* node)
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (214027 => 214028)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2017-03-16 04:10:53 UTC (rev 214027)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2017-03-16 04:49:47 UTC (rev 214028)
@@ -7925,6 +7925,61 @@
void SpeculativeJIT::compileToStringOrCallStringConstructorOnCell(Node* node)
{
+ if (node->child1().useKind() == NotCellUse) {
+ JSValueOperand op1(this, node->child1(), ManualOperandSpeculation);
+ JSValueRegs op1Regs = op1.jsValueRegs();
+
+ GPRFlushedCallResult result(this);
+ GPRReg resultGPR = result.gpr();
+
+ speculateNotCell(node->child1(), op1Regs);
+
+ flushRegisters();
+
+ if (node->op() == ToString)
+ callOperation(operationToString, resultGPR, op1Regs);
+ else {
+ ASSERT(node->op() == CallStringConstructor);
+ callOperation(operationCallStringConstructor, resultGPR, op1Regs);
+ }
+ m_jit.exceptionCheck();
+ cellResult(resultGPR, node);
+ return;
+ }
+
+ if (node->child1().useKind() == UntypedUse) {
+ JSValueOperand op1(this, node->child1());
+ JSValueRegs op1Regs = op1.jsValueRegs();
+ GPRReg op1PayloadGPR = op1Regs.payloadGPR();
+
+ GPRFlushedCallResult result(this);
+ GPRReg resultGPR = result.gpr();
+
+ flushRegisters();
+
+ JITCompiler::Jump done;
+ if (node->child1()->prediction() & SpecString) {
+ JITCompiler::Jump slowPath1 = m_jit.branchIfNotCell(op1.jsValueRegs());
+ JITCompiler::Jump slowPath2 = m_jit.branchIfNotString(op1PayloadGPR);
+ m_jit.move(op1PayloadGPR, resultGPR);
+ done = m_jit.jump();
+ slowPath1.link(&m_jit);
+ slowPath2.link(&m_jit);
+ }
+ if (node->op() == ToString)
+ callOperation(operationToString, resultGPR, op1Regs);
+ else {
+ ASSERT(node->op() == CallStringConstructor);
+ callOperation(operationCallStringConstructor, resultGPR, op1Regs);
+ }
+ m_jit.exceptionCheck();
+ if (done.isSet())
+ done.link(&m_jit);
+ cellResult(resultGPR, node);
+ return;
+ }
+
+
SpeculateCellOperand op1(this, node->child1());
GPRReg op1GPR = op1.gpr();
@@ -8533,6 +8588,11 @@
speculateSymbol(edge, operand.gpr());
}
+void SpeculativeJIT::speculateNotCell(Edge edge, JSValueRegs regs)
+{
+ DFG_TYPE_CHECK(regs, edge, ~SpecCell, m_jit.branchIfCell(regs));
+}
+
void SpeculativeJIT::speculateNotCell(Edge edge)
{
if (!needsTypeCheck(edge, ~SpecCell))
@@ -8539,7 +8599,7 @@
return;
JSValueOperand operand(this, edge, ManualOperandSpeculation);
- typeCheck(operand.jsValueRegs(), edge, ~SpecCell, m_jit.branchIfCell(operand.jsValueRegs()));
+ speculateNotCell(edge, operand.jsValueRegs());
}
void SpeculativeJIT::speculateOther(Edge edge)
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (214027 => 214028)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2017-03-16 04:10:53 UTC (rev 214027)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2017-03-16 04:49:47 UTC (rev 214028)
@@ -1644,6 +1644,11 @@
m_jit.setupArgumentsWithExecState(arg1);
return appendCallSetResult(operation, result);
}
+ JITCompiler::Call callOperation(C_JITOperation_EJ operation, GPRReg result, JSValueRegs arg1)
+ {
+ m_jit.setupArgumentsWithExecState(arg1.gpr());
+ return appendCallSetResult(operation, result);
+ }
JITCompiler::Call callOperation(C_JITOperation_EJJ operation, GPRReg result, GPRReg arg1, GPRReg arg2)
{
m_jit.setupArgumentsWithExecState(arg1, arg2);
@@ -2954,6 +2959,7 @@
void speculateStringOrStringObject(Edge);
void speculateSymbol(Edge, GPRReg cell);
void speculateSymbol(Edge);
+ void speculateNotCell(Edge, JSValueRegs);
void speculateNotCell(Edge);
void speculateOther(Edge);
void speculateMisc(Edge, JSValueRegs);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (214027 => 214028)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2017-03-16 04:10:53 UTC (rev 214027)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2017-03-16 04:49:47 UTC (rev 214028)
@@ -3769,38 +3769,6 @@
case ToString:
case CallStringConstructor: {
- if (node->child1().useKind() == UntypedUse) {
- JSValueOperand op1(this, node->child1());
- GPRReg op1PayloadGPR = op1.payloadGPR();
- JSValueRegs op1Regs = op1.jsValueRegs();
-
- GPRFlushedCallResult result(this);
- GPRReg resultGPR = result.gpr();
-
- flushRegisters();
-
- JITCompiler::Jump done;
- if (node->child1()->prediction() & SpecString) {
- JITCompiler::Jump slowPath1 = m_jit.branchIfNotCell(op1.jsValueRegs());
- JITCompiler::Jump slowPath2 = m_jit.branchIfNotString(op1PayloadGPR);
- m_jit.move(op1PayloadGPR, resultGPR);
- done = m_jit.jump();
- slowPath1.link(&m_jit);
- slowPath2.link(&m_jit);
- }
- if (op == ToString)
- callOperation(operationToString, resultGPR, op1Regs);
- else {
- ASSERT(op == CallStringConstructor);
- callOperation(operationCallStringConstructor, resultGPR, op1Regs);
- }
- m_jit.exceptionCheck();
- if (done.isSet())
- done.link(&m_jit);
- cellResult(resultGPR, node);
- break;
- }
-
compileToStringOrCallStringConstructorOnCell(node);
break;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (214027 => 214028)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2017-03-16 04:10:53 UTC (rev 214027)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2017-03-16 04:49:47 UTC (rev 214028)
@@ -3739,37 +3739,6 @@
case ToString:
case CallStringConstructor: {
- if (node->child1().useKind() == UntypedUse) {
- JSValueOperand op1(this, node->child1());
- GPRReg op1GPR = op1.gpr();
-
- GPRFlushedCallResult result(this);
- GPRReg resultGPR = result.gpr();
-
- flushRegisters();
-
- JITCompiler::Jump done;
- if (node->child1()->prediction() & SpecString) {
- JITCompiler::Jump slowPath1 = m_jit.branchIfNotCell(JSValueRegs(op1GPR));
- JITCompiler::Jump slowPath2 = m_jit.branchIfNotString(op1GPR);
- m_jit.move(op1GPR, resultGPR);
- done = m_jit.jump();
- slowPath1.link(&m_jit);
- slowPath2.link(&m_jit);
- }
- if (op == ToString)
- callOperation(operationToString, resultGPR, op1GPR);
- else {
- ASSERT(op == CallStringConstructor);
- callOperation(operationCallStringConstructor, resultGPR, op1GPR);
- }
- m_jit.exceptionCheck();
- if (done.isSet())
- done.link(&m_jit);
- cellResult(resultGPR, node);
- break;
- }
-
compileToStringOrCallStringConstructorOnCell(node);
break;
}
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (214027 => 214028)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-03-16 04:10:53 UTC (rev 214027)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-03-16 04:49:47 UTC (rev 214028)
@@ -4933,10 +4933,13 @@
}
case CellUse:
+ case NotCellUse:
case UntypedUse: {
LValue value;
if (m_node->child1().useKind() == CellUse)
value = lowCell(m_node->child1());
+ else if (m_node->child1().useKind() == NotCellUse)
+ value = lowNotCell(m_node->child1());
else
value = lowJSValue(m_node->child1());
@@ -4947,6 +4950,8 @@
LValue isCellPredicate;
if (m_node->child1().useKind() == CellUse)
isCellPredicate = m_out.booleanTrue;
+ else if (m_node->child1().useKind() == NotCellUse)
+ isCellPredicate = m_out.booleanFalse;
else
isCellPredicate = this->isCell(value, provenType(m_node->child1()));
m_out.branch(isCellPredicate, unsure(isCell), unsure(notString));
@@ -12228,6 +12233,13 @@
DFG_CRASH(m_graph, m_node, "Value not defined");
return 0;
}
+
+ LValue lowNotCell(Edge edge)
+ {
+ LValue result = lowJSValue(edge, ManualOperandSpeculation);
+ FTL_TYPE_CHECK(jsValueValue(result), edge, ~SpecCell, isCell(result));
+ return result;
+ }
LValue lowStorage(Edge edge)
{
@@ -12632,6 +12644,13 @@
{
lowCell(edge);
}
+
+ void speculateNotCell(Edge edge)
+ {
+ if (!m_interpreter.needsTypeCheck(edge))
+ return;
+ lowNotCell(edge);
+ }
void speculateCellOrOther(Edge edge)
{
@@ -13150,15 +13169,6 @@
m_out.appendTo(continuation, lastNext);
}
- void speculateNotCell(Edge edge)
- {
- if (!m_interpreter.needsTypeCheck(edge))
- return;
-
- LValue value = lowJSValue(edge, ManualOperandSpeculation);
- typeCheck(jsValueValue(value), edge, ~SpecCell, isCell(value));
- }
-
void speculateOther(Edge edge)
{
if (!m_interpreter.needsTypeCheck(edge))
