DFG should be able to inline string equality comparisons
https://bugs.webkit.org/show_bug.cgi?id=114224
Source/JavaScriptCore:
Reviewed by Oliver Hunt.
Inline 8-bit string equality, go to slow path for 16-bit strings. 2x speed-up for string equality
comparisons on 8-bit strings. 20-50% speed-up on JSRegress/HashMap tests. 30% speed-up on
string-fasta. 2% speed-up on SunSpider overall. Some small speed-ups elsewhere.
This is a gnarly change but we have loads of test coverage already between the HashMap tests and
preexisting DFG string equality tests (which appear to have been designed to test OSR exits, but
also give us good overall coverage on string equality behavior).
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGOperations.cpp:
* dfg/DFGOperations.h:
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compilePeepHoleBranch):
(JSC::DFG::SpeculativeJIT::compare):
(JSC::DFG::SpeculativeJIT::compileStrictEq):
(JSC::DFG::SpeculativeJIT::compileStringEquality):
(DFG):
* dfg/DFGSpeculativeJIT.h:
(SpeculativeJIT):
LayoutTests:
Reviewed by Oliver Hunt.
* fast/js/regress/script-tests/string-equality.js: Added.
(foo):
* fast/js/regress/string-equality-expected.txt: Added.
* fast/js/regress/string-equality.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@147965 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index 4debbac..3a15c30 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -284,14 +284,15 @@
break;
}
+ case CompareEqConstant: {
+ break;
+ }
+
case CompareEq:
- case CompareEqConstant:
case CompareLess:
case CompareLessEq:
case CompareGreater:
case CompareGreaterEq: {
- if (node->op() == CompareEqConstant)
- break;
if (Node::shouldSpeculateInteger(node->child1().node(), node->child2().node())) {
setUseKindAndUnboxIfProfitable<Int32Use>(node->child1());
setUseKindAndUnboxIfProfitable<Int32Use>(node->child2());
@@ -304,8 +305,11 @@
}
if (node->op() != CompareEq)
break;
- if (node->child1()->shouldSpeculateString() && node->child2()->shouldSpeculateString())
+ if (node->child1()->shouldSpeculateString() && node->child2()->shouldSpeculateString() && GPRInfo::numberOfRegisters >= 7) {
+ setUseKindAndUnboxIfProfitable<StringUse>(node->child1());
+ setUseKindAndUnboxIfProfitable<StringUse>(node->child2());
break;
+ }
if (node->child1()->shouldSpeculateObject() && node->child2()->shouldSpeculateObject()) {
setUseKindAndUnboxIfProfitable<ObjectUse>(node->child1());
setUseKindAndUnboxIfProfitable<ObjectUse>(node->child2());
@@ -324,10 +328,11 @@
break;
}
- case CompareStrictEq:
case CompareStrictEqConstant: {
- if (node->op() == CompareStrictEqConstant)
- break;
+ break;
+ }
+
+ case CompareStrictEq: {
if (Node::shouldSpeculateInteger(node->child1().node(), node->child2().node())) {
setUseKindAndUnboxIfProfitable<Int32Use>(node->child1());
setUseKindAndUnboxIfProfitable<Int32Use>(node->child2());
@@ -338,8 +343,11 @@
fixDoubleEdge<NumberUse>(node->child2());
break;
}
- if (node->child1()->shouldSpeculateString() && node->child2()->shouldSpeculateString())
+ if (node->child1()->shouldSpeculateString() && node->child2()->shouldSpeculateString() && GPRInfo::numberOfRegisters >= 7) {
+ setUseKindAndUnboxIfProfitable<StringUse>(node->child1());
+ setUseKindAndUnboxIfProfitable<StringUse>(node->child2());
break;
+ }
if (node->child1()->shouldSpeculateObject() && node->child2()->shouldSpeculateObject()) {
setUseKindAndUnboxIfProfitable<ObjectUse>(node->child1());
setUseKindAndUnboxIfProfitable<ObjectUse>(node->child2());
diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp
index b2025a6..2765e90 100644
--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
@@ -995,6 +995,23 @@
return JSValue::equalSlowCaseInline(exec, JSValue::decode(encodedOp1), JSValue::decode(encodedOp2));
}
+#if USE(JSVALUE64)
+EncodedJSValue DFG_OPERATION operationCompareStringEq(ExecState* exec, JSCell* left, JSCell* right)
+#else
+size_t DFG_OPERATION operationCompareStringEq(ExecState* exec, JSCell* left, JSCell* right)
+#endif
+{
+ JSGlobalData* globalData = &exec->globalData();
+ NativeCallFrameTracer tracer(globalData, exec);
+
+ bool result = asString(left)->value(exec) == asString(right)->value(exec);
+#if USE(JSVALUE64)
+ return JSValue::encode(jsBoolean(result));
+#else
+ return result;
+#endif
+}
+
size_t DFG_OPERATION operationCompareStrictEqCell(ExecState* exec, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2)
{
JSGlobalData* globalData = &exec->globalData();
diff --git a/Source/JavaScriptCore/dfg/DFGOperations.h b/Source/JavaScriptCore/dfg/DFGOperations.h
index f8c4907..b664812 100644
--- a/Source/JavaScriptCore/dfg/DFGOperations.h
+++ b/Source/JavaScriptCore/dfg/DFGOperations.h
@@ -183,6 +183,11 @@
size_t DFG_OPERATION operationCompareGreater(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
size_t DFG_OPERATION operationCompareGreaterEq(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
size_t DFG_OPERATION operationCompareEq(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
+#if USE(JSVALUE64)
+EncodedJSValue DFG_OPERATION operationCompareStringEq(ExecState*, JSCell* left, JSCell* right) WTF_INTERNAL;
+#else
+size_t DFG_OPERATION operationCompareStringEq(ExecState*, JSCell* left, JSCell* right) WTF_INTERNAL;
+#endif
size_t DFG_OPERATION operationCompareStrictEqCell(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
size_t DFG_OPERATION operationCompareStrictEq(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
char* DFG_OPERATION operationVirtualCall(ExecState*) WTF_INTERNAL;
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index f773903..1b19af6 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -1518,8 +1518,8 @@
compilePeepHoleDoubleBranch(node, branchNode, doubleCondition);
else if (node->op() == CompareEq) {
if (node->isBinaryUseKind(StringUse)) {
- nonSpeculativePeepholeBranch(node, branchNode, condition, operation);
- return true;
+ // Use non-peephole comparison, for now.
+ return false;
}
if (node->isBinaryUseKind(ObjectUse))
compilePeepHoleObjectEquality(node, branchNode);
@@ -3476,7 +3476,7 @@
if (node->op() == CompareEq) {
if (node->isBinaryUseKind(StringUse)) {
- nonSpeculativeNonPeepholeCompare(node, condition, operation);
+ compileStringEquality(node);
return false;
}
@@ -3610,7 +3610,8 @@
}
case StringUse: {
- return nonSpeculativeStrictEq(node);
+ compileStringEquality(node);
+ return false;
}
case ObjectUse: {
@@ -3638,6 +3639,111 @@
}
}
+void SpeculativeJIT::compileStringEquality(Node* node)
+{
+ SpeculateCellOperand left(this, node->child1());
+ SpeculateCellOperand right(this, node->child2());
+ GPRTemporary length(this);
+ GPRTemporary leftTemp(this);
+ GPRTemporary rightTemp(this);
+ GPRTemporary leftTemp2(this, left);
+ GPRTemporary rightTemp2(this, right);
+
+ GPRReg leftGPR = left.gpr();
+ GPRReg rightGPR = right.gpr();
+ GPRReg lengthGPR = length.gpr();
+ GPRReg leftTempGPR = leftTemp.gpr();
+ GPRReg rightTempGPR = rightTemp.gpr();
+ GPRReg leftTemp2GPR = leftTemp2.gpr();
+ GPRReg rightTemp2GPR = rightTemp2.gpr();
+
+ JITCompiler::JumpList trueCase;
+ JITCompiler::JumpList falseCase;
+ JITCompiler::JumpList slowCase;
+
+ DFG_TYPE_CHECK(
+ JSValueSource::unboxedCell(leftGPR), node->child1(), SpecString, m_jit.branchPtr(
+ MacroAssembler::NotEqual,
+ MacroAssembler::Address(leftGPR, JSCell::structureOffset()),
+ MacroAssembler::TrustedImmPtr(m_jit.globalData()->stringStructure.get())));
+
+ // It's safe to branch around the type check below, since proving that the values are
+ // equal does indeed prove that the right value is a string.
+ trueCase.append(m_jit.branchPtr(MacroAssembler::Equal, leftGPR, rightGPR));
+
+ DFG_TYPE_CHECK(
+ JSValueSource::unboxedCell(rightGPR), node->child2(), SpecString, m_jit.branchPtr(
+ MacroAssembler::NotEqual,
+ MacroAssembler::Address(rightGPR, JSCell::structureOffset()),
+ MacroAssembler::TrustedImmPtr(m_jit.globalData()->stringStructure.get())));
+
+ m_jit.load32(MacroAssembler::Address(leftGPR, JSString::offsetOfLength()), lengthGPR);
+
+ falseCase.append(m_jit.branch32(
+ MacroAssembler::NotEqual,
+ MacroAssembler::Address(rightGPR, JSString::offsetOfLength()),
+ lengthGPR));
+
+ trueCase.append(m_jit.branchTest32(MacroAssembler::Zero, lengthGPR));
+
+ m_jit.loadPtr(MacroAssembler::Address(leftGPR, JSString::offsetOfValue()), leftTempGPR);
+ m_jit.loadPtr(MacroAssembler::Address(rightGPR, JSString::offsetOfValue()), rightTempGPR);
+
+ slowCase.append(m_jit.branchTestPtr(MacroAssembler::Zero, leftTempGPR));
+ slowCase.append(m_jit.branchTestPtr(MacroAssembler::Zero, rightTempGPR));
+
+ slowCase.append(m_jit.branchTest32(
+ MacroAssembler::Zero,
+ MacroAssembler::Address(leftTempGPR, StringImpl::flagsOffset()),
+ TrustedImm32(StringImpl::flagIs8Bit())));
+ slowCase.append(m_jit.branchTest32(
+ MacroAssembler::Zero,
+ MacroAssembler::Address(rightTempGPR, StringImpl::flagsOffset()),
+ TrustedImm32(StringImpl::flagIs8Bit())));
+
+ m_jit.loadPtr(MacroAssembler::Address(leftTempGPR, StringImpl::dataOffset()), leftTempGPR);
+ m_jit.loadPtr(MacroAssembler::Address(rightTempGPR, StringImpl::dataOffset()), rightTempGPR);
+
+ MacroAssembler::Label loop = m_jit.label();
+
+ m_jit.sub32(TrustedImm32(1), lengthGPR);
+
+ // This isn't going to generate the best code on x86. But that's OK, it's still better
+ // than not inlining.
+ m_jit.load8(MacroAssembler::BaseIndex(leftTempGPR, lengthGPR, MacroAssembler::TimesOne), leftTemp2GPR);
+ m_jit.load8(MacroAssembler::BaseIndex(rightTempGPR, lengthGPR, MacroAssembler::TimesOne), rightTemp2GPR);
+ falseCase.append(m_jit.branch32(MacroAssembler::NotEqual, leftTemp2GPR, rightTemp2GPR));
+
+ m_jit.branchTest32(MacroAssembler::NonZero, lengthGPR).linkTo(loop, &m_jit);
+
+ trueCase.link(&m_jit);
+#if USE(JSVALUE64)
+ m_jit.move(TrustedImm64(ValueTrue), leftTempGPR);
+#else
+ m_jit.move(TrustedImm32(true), leftTempGPR);
+#endif
+
+ JITCompiler::Jump done = m_jit.jump();
+
+ falseCase.link(&m_jit);
+#if USE(JSVALUE64)
+ m_jit.move(TrustedImm64(ValueFalse), leftTempGPR);
+#else
+ m_jit.move(TrustedImm32(false), leftTempGPR);
+#endif
+
+ done.link(&m_jit);
+ addSlowPathGenerator(
+ slowPathCall(
+ slowCase, this, operationCompareStringEq, leftTempGPR, leftGPR, rightGPR));
+
+#if USE(JSVALUE64)
+ jsValueResult(leftTempGPR, node, DataFormatJSBoolean);
+#else
+ booleanResult(leftTempGPR, node);
+#endif
+}
+
void SpeculativeJIT::compileGetIndexedPropertyStorage(Node* node)
{
SpeculateCellOperand base(this, node->child1());
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
index 352f28e..2359ae0 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
@@ -2071,6 +2071,7 @@
void compileValueAdd(Node*);
void compileObjectOrOtherLogicalNot(Edge value);
void compileLogicalNot(Node*);
+ void compileStringEquality(Node*);
void emitObjectOrOtherBranch(Edge value, BlockIndex taken, BlockIndex notTaken);
void emitBranch(Node*);
