Skip to content

Commit bd3aa0e

Browse files
sosukesuzukisosukesuzuki
committed
[JSC] Compare lengths inline before calling operationCompareStringEq for ropes
https://bugs.webkit.org/show_bug.cgi?id=326312 Reviewed by Yusuke Suzuki. When either operand of a string equality is a rope, DFG and FTL always take the slow path and call operationCompareStringEq. If the lengths differ, the operation returns false without resolving the rope, so the same comparison takes the slow path every time. This patch compares the lengths in the rope case first. If they differ, the result is false and no call is made. Otherwise we call the operation as before. * Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp: (JSC::DFG::SpeculativeJIT::compileStringEquality): * Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq): Canonical link: https://commits.webkit.org/322690@main
1 parent 253c954 commit bd3aa0e

2 files changed

Lines changed: 48 additions & 5 deletions

File tree

‎Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7341,6 +7341,7 @@ void SpeculativeJIT::compileStringEquality(
73417341
JumpList trueCase;
73427342
JumpList falseCase;
73437343
JumpList slowCase;
7344+
JumpList ropeCase;
73447345

73457346
trueCase.append(fastTrue);
73467347
falseCase.append(fastFalse);
@@ -7349,7 +7350,7 @@ void SpeculativeJIT::compileStringEquality(
73497350
if (atom)
73507351
return;
73517352
loadPtr(Address(jsStringGPR, JSString::offsetOfValue()), implGPR);
7352-
slowCase.append(branchIfRopeStringImpl(implGPR));
7353+
ropeCase.append(branchIfRopeStringImpl(implGPR));
73537354
};
73547355

73557356
auto resolveDataPtr = [&](bool atom, const String& constStr, GPRReg implGPR) {
@@ -7455,14 +7456,30 @@ void SpeculativeJIT::compileStringEquality(
74557456
Jump done = jump();
74567457

74577458
falseCase.link(this);
7459+
Label falseResult = label();
74587460
moveFalseTo(leftTempGPR);
74597461

74607462
done.link(this);
74617463

74627464
Vector<SilentRegisterSavePlan> savePlans;
74637465
silentSpillAllRegistersImpl(false, savePlans, leftTempGPR);
74647466
Label doneOperationCall = label();
7465-
addSlowPathGeneratorLambda([=, this, savePlans = WTF::move(savePlans), slowCase = WTF::move(slowCase)]() mutable {
7467+
addSlowPathGeneratorLambda([=, this, savePlans = WTF::move(savePlans), slowCase = WTF::move(slowCase), ropeCase = WTF::move(ropeCase)]() mutable {
7468+
auto loadLength = [&](GPRReg jsStringGPR, GPRReg resultGPR) {
7469+
loadPtr(Address(jsStringGPR, JSString::offsetOfValue()), resultGPR);
7470+
Jump isRope = branchIfRopeStringImpl(resultGPR);
7471+
load32(Address(resultGPR, StringImpl::lengthMemoryOffset()), resultGPR);
7472+
Jump lengthLoaded = jump();
7473+
isRope.link(this);
7474+
load32(Address(jsStringGPR, JSRopeString::offsetOfLength()), resultGPR);
7475+
lengthLoaded.link(this);
7476+
};
7477+
7478+
ropeCase.link(this);
7479+
loadLength(leftGPR, leftTempGPR);
7480+
loadLength(rightGPR, rightTempGPR);
7481+
branch32(NotEqual, leftTempGPR, rightTempGPR).linkTo(falseResult, this);
7482+
74667483
slowCase.link(this);
74677484
silentSpill(savePlans);
74687485
setupArguments<decltype(operationCompareStringEq)>(LinkableConstant::globalObject(*this, node), leftGPR, rightGPR);

‎Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22733,19 +22733,20 @@ IGNORE_CLANG_WARNINGS_END
2273322733
LBasicBlock wordTailCompare = m_out.newBlock();
2273422734
LBasicBlock trueCase = m_out.newBlock();
2273522735
LBasicBlock falseCase = m_out.newBlock();
22736+
LBasicBlock ropeCase = m_out.newBlock();
2273622737
LBasicBlock slowCase = m_out.newBlock();
2273722738
LBasicBlock continuation = m_out.newBlock();
2273822739

2273922740
if (leftAtom)
2274022741
m_out.jump(leftReadyCase);
2274122742
else
22742-
m_out.branch(isRopeString(leftJSString, leftJSStringEdge), rarely(slowCase), usually(leftReadyCase));
22743+
m_out.branch(isRopeString(leftJSString, leftJSStringEdge), rarely(ropeCase), usually(leftReadyCase));
2274322744

2274422745
LBasicBlock lastNext = m_out.appendTo(leftReadyCase, rightReadyCase);
2274522746
if (rightAtom)
2274622747
m_out.jump(rightReadyCase);
2274722748
else
22748-
m_out.branch(isRopeString(rightJSString, rightJSStringEdge), rarely(slowCase), usually(rightReadyCase));
22749+
m_out.branch(isRopeString(rightJSString, rightJSStringEdge), rarely(ropeCase), usually(rightReadyCase));
2274922750

2275022751
m_out.appendTo(rightReadyCase, notTriviallyUnequalCase);
2275122752
LValue left = leftAtom ? nullptr : m_out.loadPtr(leftJSString, m_heaps.JSString_value);
@@ -22856,11 +22857,36 @@ IGNORE_CLANG_WARNINGS_END
2285622857
ValueFromBlock trueResult = m_out.anchor(m_out.booleanTrue);
2285722858
m_out.jump(continuation);
2285822859

22859-
m_out.appendTo(falseCase, slowCase);
22860+
m_out.appendTo(falseCase, ropeCase);
2286022861

2286122862
ValueFromBlock falseResult = m_out.anchor(m_out.booleanFalse);
2286222863
m_out.jump(continuation);
2286322864

22865+
m_out.appendTo(ropeCase, slowCase);
22866+
22867+
auto loadLength = [&](LValue jsString, Edge edge) {
22868+
LBasicBlock ropePath = m_out.newBlock();
22869+
LBasicBlock nonRopePath = m_out.newBlock();
22870+
LBasicBlock lengthLoaded = m_out.newBlock();
22871+
22872+
m_out.branch(isRopeString(jsString, edge), unsure(ropePath), unsure(nonRopePath));
22873+
22874+
m_out.appendTo(ropePath, nonRopePath);
22875+
ValueFromBlock ropeLength = m_out.anchor(m_out.load32NonNegative(jsString, m_heaps.JSRopeString_length));
22876+
m_out.jump(lengthLoaded);
22877+
22878+
m_out.appendTo(nonRopePath, lengthLoaded);
22879+
ValueFromBlock nonRopeLength = m_out.anchor(m_out.load32NonNegative(m_out.loadPtr(jsString, m_heaps.JSString_value), m_heaps.StringImpl_length));
22880+
m_out.jump(lengthLoaded);
22881+
22882+
m_out.appendTo(lengthLoaded, slowCase);
22883+
return m_out.phi(Int32, ropeLength, nonRopeLength);
22884+
};
22885+
22886+
LValue leftRopeCaseLength = loadLength(leftJSString, leftJSStringEdge);
22887+
LValue rightRopeCaseLength = loadLength(rightJSString, rightJSStringEdge);
22888+
m_out.branch(m_out.notEqual(leftRopeCaseLength, rightRopeCaseLength), usually(falseCase), rarely(slowCase));
22889+
2286422890
m_out.appendTo(slowCase, continuation);
2286522891

2286622892
VM* vmPtr = &vm();

0 commit comments

Comments
 (0)