Introduce SymbolUse optimization into CompareEq and CompareStrictEq
https://bugs.webkit.org/show_bug.cgi?id=149616
Reviewed by Saam Barati.
Since ES6 Symbols are used as an enum value[1] (And WebKit inspector do so for Esprima's type of nodes),
optimizing equality comparison for symbols makes much sense.
This patch leverages SymbolUse for CompareEq and CompareStrictEq.
Optimizations for both DFG and FTL are implemented.
[1]: http://www.2ality.com/2014/12/es6-symbols.html
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGNode.h:
(JSC::DFG::Node::shouldSpeculateSymbol):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compilePeepHoleBranch):
(JSC::DFG::SpeculativeJIT::compare):
(JSC::DFG::SpeculativeJIT::compileStrictEq):
(JSC::DFG::SpeculativeJIT::extractStringImplFromBinarySymbols):
(JSC::DFG::SpeculativeJIT::compileSymbolEquality):
(JSC::DFG::SpeculativeJIT::compilePeepHoleSymbolEquality):
* dfg/DFGSpeculativeJIT.h:
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::compileCompareEq):
(JSC::FTL::DFG::LowerDFGToLLVM::compileCompareStrictEq):
* tests/stress/symbol-equality.js: Added.
(shouldBe):
(equal):
(strictEqual):
(list.forEach.result.set 1):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@190414 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index a0fc90b..60b8f85 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,43 @@
+2015-10-01 Yusuke Suzuki <utatane.tea@gmail.com>
+
+ Introduce SymbolUse optimization into CompareEq and CompareStrictEq
+ https://bugs.webkit.org/show_bug.cgi?id=149616
+
+ Reviewed by Saam Barati.
+
+ Since ES6 Symbols are used as an enum value[1] (And WebKit inspector do so for Esprima's type of nodes),
+ optimizing equality comparison for symbols makes much sense.
+
+ This patch leverages SymbolUse for CompareEq and CompareStrictEq.
+ Optimizations for both DFG and FTL are implemented.
+
+ [1]: http://www.2ality.com/2014/12/es6-symbols.html
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGNode.h:
+ (JSC::DFG::Node::shouldSpeculateSymbol):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compilePeepHoleBranch):
+ (JSC::DFG::SpeculativeJIT::compare):
+ (JSC::DFG::SpeculativeJIT::compileStrictEq):
+ (JSC::DFG::SpeculativeJIT::extractStringImplFromBinarySymbols):
+ (JSC::DFG::SpeculativeJIT::compileSymbolEquality):
+ (JSC::DFG::SpeculativeJIT::compilePeepHoleSymbolEquality):
+ * dfg/DFGSpeculativeJIT.h:
+ * ftl/FTLCapabilities.cpp:
+ (JSC::FTL::canCompile):
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileCompareEq):
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileCompareStrictEq):
+ * tests/stress/symbol-equality.js: Added.
+ (shouldBe):
+ (equal):
+ (strictEqual):
+ (list.forEach.result.set 1):
+
2015-10-01 Youenn Fablet <youenn.fablet@crf.canon.fr>
[Streams API] Add support for private WebCore JS builtins functions
diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
index d1677c6..69183f7 100644
--- a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
+++ b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
@@ -1188,6 +1188,11 @@
break;
}
}
+
+ if (node->op() == CompareEq && leftConst.isSymbol() && rightConst.isSymbol()) {
+ setConstant(node, jsBoolean(asSymbol(leftConst)->privateName() == asSymbol(rightConst)->privateName()));
+ break;
+ }
}
if (node->op() == CompareEq) {
@@ -1231,6 +1236,7 @@
node->isBinaryUseKind(Int52RepUse) ||
node->isBinaryUseKind(StringUse) ||
node->isBinaryUseKind(BooleanUse) ||
+ node->isBinaryUseKind(SymbolUse) ||
node->isBinaryUseKind(StringIdentUse) ||
node->isBinaryUseKind(ObjectUse) ||
node->isBinaryUseKind(ObjectUse, ObjectOrOtherUse) ||
@@ -1291,6 +1297,7 @@
node->isBinaryUseKind(Int52RepUse) ||
node->isBinaryUseKind(StringUse) ||
node->isBinaryUseKind(StringIdentUse) ||
+ node->isBinaryUseKind(SymbolUse) ||
node->isBinaryUseKind(ObjectUse) ||
node->isBinaryUseKind(MiscUse, UntypedUse) ||
node->isBinaryUseKind(UntypedUse, MiscUse) ||
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index d5e9c3d..790ca75 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -411,6 +411,12 @@
}
if (node->op() != CompareEq)
break;
+ if (Node::shouldSpeculateSymbol(node->child1().node(), node->child2().node())) {
+ fixEdge<SymbolUse>(node->child1());
+ fixEdge<SymbolUse>(node->child2());
+ node->clearFlags(NodeMustGenerate);
+ break;
+ }
if (node->child1()->shouldSpeculateStringIdent() && node->child2()->shouldSpeculateStringIdent()) {
fixEdge<StringIdentUse>(node->child1());
fixEdge<StringIdentUse>(node->child2());
@@ -493,6 +499,11 @@
fixEdge<DoubleRepUse>(node->child2());
break;
}
+ if (Node::shouldSpeculateSymbol(node->child1().node(), node->child2().node())) {
+ fixEdge<SymbolUse>(node->child1());
+ fixEdge<SymbolUse>(node->child2());
+ break;
+ }
if (node->child1()->shouldSpeculateStringIdent() && node->child2()->shouldSpeculateStringIdent()) {
fixEdge<StringIdentUse>(node->child1());
fixEdge<StringIdentUse>(node->child2());
diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h
index 794ef3d..f1a0ed1 100644
--- a/Source/JavaScriptCore/dfg/DFGNode.h
+++ b/Source/JavaScriptCore/dfg/DFGNode.h
@@ -1876,6 +1876,11 @@
{
return isStringOrStringObjectSpeculation(prediction());
}
+
+ bool shouldSpeculateSymbol()
+ {
+ return isSymbolSpeculation(prediction());
+ }
bool shouldSpeculateFinalObject()
{
@@ -2021,6 +2026,11 @@
return op1->shouldSpeculateNumberOrBooleanExpectingDefined()
&& op2->shouldSpeculateNumberOrBooleanExpectingDefined();
}
+
+ static bool shouldSpeculateSymbol(Node* op1, Node* op2)
+ {
+ return op1->shouldSpeculateSymbol() && op2->shouldSpeculateSymbol();
+ }
static bool shouldSpeculateFinalObject(Node* op1, Node* op2)
{
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index 096f25b..58d434c 100755
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -1356,6 +1356,8 @@
}
if (node->isBinaryUseKind(BooleanUse))
compilePeepHoleBooleanBranch(node, branchNode, condition);
+ else if (node->isBinaryUseKind(SymbolUse))
+ compilePeepHoleSymbolEquality(node, branchNode);
else if (node->isBinaryUseKind(ObjectUse))
compilePeepHoleObjectEquality(node, branchNode);
else if (node->isBinaryUseKind(ObjectUse, ObjectOrOtherUse))
@@ -3879,6 +3881,11 @@
compileStringIdentEquality(node);
return false;
}
+
+ if (node->isBinaryUseKind(SymbolUse)) {
+ compileSymbolEquality(node);
+ return false;
+ }
if (node->isBinaryUseKind(ObjectUse)) {
compileObjectEquality(node);
@@ -3912,6 +3919,12 @@
bool SpeculativeJIT::compileStrictEq(Node* node)
{
+ // FIXME: Currently, we have op_jless, op_jgreater etc. But we don't have op_jeq, op_jstricteq etc.
+ // `==` and `===` operations with branching will be compiled to op_{eq,stricteq} and op_{jfalse,jtrue}.
+ // In DFG bytecodes, between op_eq and op_jfalse, we have MovHint to store the result of op_eq.
+ // As a result, detectPeepHoleBranch() never detects peep hole for that case.
+ // https://bugs.webkit.org/show_bug.cgi?id=149713
+
if (node->isBinaryUseKind(BooleanUse)) {
unsigned branchIndexInBlock = detectPeepHoleBranch();
if (branchIndexInBlock != UINT_MAX) {
@@ -3973,6 +3986,21 @@
compileDoubleCompare(node, MacroAssembler::DoubleEqual);
return false;
}
+
+ if (node->isBinaryUseKind(SymbolUse)) {
+ unsigned branchIndexInBlock = detectPeepHoleBranch();
+ if (branchIndexInBlock != UINT_MAX) {
+ Node* branchNode = m_block->at(branchIndexInBlock);
+ compilePeepHoleSymbolEquality(node, branchNode);
+ use(node->child1());
+ use(node->child2());
+ m_indexInBlock = branchIndexInBlock;
+ m_currentNode = branchNode;
+ return true;
+ }
+ compileSymbolEquality(node);
+ return false;
+ }
if (node->isBinaryUseKind(StringUse)) {
compileStringEquality(node);
@@ -4070,6 +4098,52 @@
unblessedBooleanResult(result.gpr(), node);
}
+template<typename Functor>
+void SpeculativeJIT::extractStringImplFromBinarySymbols(Edge leftSymbolEdge, Edge rightSymbolEdge, const Functor& functor)
+{
+ SpeculateCellOperand left(this, leftSymbolEdge);
+ SpeculateCellOperand right(this, rightSymbolEdge);
+ GPRTemporary leftTemp(this);
+ GPRTemporary rightTemp(this);
+
+ GPRReg leftGPR = left.gpr();
+ GPRReg rightGPR = right.gpr();
+ GPRReg leftTempGPR = leftTemp.gpr();
+ GPRReg rightTempGPR = rightTemp.gpr();
+
+ speculateSymbol(leftSymbolEdge, leftGPR);
+ speculateSymbol(rightSymbolEdge, rightGPR);
+
+ m_jit.loadPtr(JITCompiler::Address(leftGPR, Symbol::offsetOfPrivateName()), leftTempGPR);
+ m_jit.loadPtr(JITCompiler::Address(rightGPR, Symbol::offsetOfPrivateName()), rightTempGPR);
+
+ functor(leftTempGPR, rightTempGPR);
+}
+
+void SpeculativeJIT::compileSymbolEquality(Node* node)
+{
+ extractStringImplFromBinarySymbols(node->child1(), node->child2(), [&] (GPRReg leftStringImpl, GPRReg rightStringImpl) {
+ m_jit.comparePtr(JITCompiler::Equal, leftStringImpl, rightStringImpl, leftStringImpl);
+ unblessedBooleanResult(leftStringImpl, node);
+ });
+}
+
+void SpeculativeJIT::compilePeepHoleSymbolEquality(Node* node, Node* branchNode)
+{
+ BasicBlock* taken = branchNode->branchData()->taken.block;
+ BasicBlock* notTaken = branchNode->branchData()->notTaken.block;
+
+ extractStringImplFromBinarySymbols(node->child1(), node->child2(), [&] (GPRReg leftStringImpl, GPRReg rightStringImpl) {
+ if (taken == nextBlock()) {
+ branchPtr(JITCompiler::NotEqual, leftStringImpl, rightStringImpl, notTaken);
+ jump(taken);
+ } else {
+ branchPtr(JITCompiler::Equal, leftStringImpl, rightStringImpl, taken);
+ jump(notTaken);
+ }
+ });
+}
+
void SpeculativeJIT::compileStringEquality(
Node* node, GPRReg leftGPR, GPRReg rightGPR, GPRReg lengthGPR, GPRReg leftTempGPR,
GPRReg rightTempGPR, GPRReg leftTemp2GPR, GPRReg rightTemp2GPR,
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
index 4812fa1..b420eee 100755
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
@@ -2094,6 +2094,11 @@
void compileStringZeroLength(Node*);
void compileMiscStrictEq(Node*);
+ template<typename Functor>
+ void extractStringImplFromBinarySymbols(Edge leftSymbolEdge, Edge rightSymbolEdge, const Functor&);
+ void compileSymbolEquality(Node*);
+ void compilePeepHoleSymbolEquality(Node*, Node* branchNode);
+
void emitObjectOrOtherBranch(Edge value, BasicBlock* taken, BasicBlock* notTaken);
void emitStringBranch(Edge value, BasicBlock* taken, BasicBlock* notTaken);
void emitBranch(Node*);
diff --git a/Source/JavaScriptCore/ftl/FTLCapabilities.cpp b/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
index f3617be..1e96e2b 100644
--- a/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
+++ b/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
@@ -320,6 +320,8 @@
break;
if (node->isBinaryUseKind(StringIdentUse))
break;
+ if (node->isBinaryUseKind(SymbolUse))
+ break;
if (node->isBinaryUseKind(ObjectUse))
break;
if (node->isBinaryUseKind(UntypedUse))
@@ -350,6 +352,8 @@
break;
if (node->isBinaryUseKind(BooleanUse))
break;
+ if (node->isBinaryUseKind(SymbolUse))
+ break;
if (node->isBinaryUseKind(MiscUse, UntypedUse))
break;
if (node->isBinaryUseKind(UntypedUse, MiscUse))
diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
index 986b435..804d30e 100644
--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
@@ -4202,6 +4202,7 @@
|| m_node->isBinaryUseKind(DoubleRepUse)
|| m_node->isBinaryUseKind(ObjectUse)
|| m_node->isBinaryUseKind(BooleanUse)
+ || m_node->isBinaryUseKind(SymbolUse)
|| m_node->isBinaryUseKind(StringIdentUse)) {
compileCompareStrictEq();
return;
@@ -4294,6 +4295,15 @@
m_out.equal(lowBoolean(m_node->child1()), lowBoolean(m_node->child2())));
return;
}
+
+ if (m_node->isBinaryUseKind(SymbolUse)) {
+ LValue left = lowSymbol(m_node->child1());
+ LValue right = lowSymbol(m_node->child2());
+ LValue leftStringImpl = m_out.loadPtr(left, m_heaps.Symbol_privateName);
+ LValue rightStringImpl = m_out.loadPtr(right, m_heaps.Symbol_privateName);
+ setBoolean(m_out.equal(leftStringImpl, rightStringImpl));
+ return;
+ }
if (m_node->isBinaryUseKind(MiscUse, UntypedUse)
|| m_node->isBinaryUseKind(UntypedUse, MiscUse)) {
diff --git a/Source/JavaScriptCore/tests/stress/symbol-equality.js b/Source/JavaScriptCore/tests/stress/symbol-equality.js
new file mode 100644
index 0000000..eb64a89
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/symbol-equality.js
@@ -0,0 +1,34 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+function equal(a, b) {
+ return a == b;
+}
+noInline(equal);
+
+function strictEqual(a, b) {
+ return a === b;
+}
+noInline(strictEqual);
+
+var s1 = Symbol()
+var s2 = Symbol();
+
+var list = [
+ [ [ s1, s1 ], true ],
+ [ [ s2, s1 ], false ],
+ [ [ s1, s2 ], false ],
+ [ [ s2, s2 ], true ],
+ [ [ s2, 42 ], false ],
+];
+
+list.forEach(function (set) {
+ var pair = set[0];
+ var result = set[1];
+ for (var i = 0; i < 10000; ++i) {
+ shouldBe(equal(pair[0], pair[1]), result);
+ shouldBe(strictEqual(pair[0], pair[1]), result);
+ }
+});
