Skip to content

Commit 23903f1

Browse files
ConstellationConstellation
committed
[JSC] Add AtomString Array loop in DFG / FTL
https://bugs.webkit.org/show_bug.cgi?id=318642 rdar://181445937 Reviewed by Yijia Huang. This patch adds fast path for indexOf / includes with AtomString arrays. If Array is an AtomString array and input is JSString with AtomString, we do pointer comparison loop. Also we use compare32 to clean up result generation in DFG code, which becomes branchless and faster. FTL already achieves it due to B3's conversion. Tests: JSTests/stress/array-indexof-includes-cow-atom-strings.js JSTests/stress/array-indexof-includes-result-boundaries.js * JSTests/stress/array-indexof-includes-cow-atom-strings.js: Added. (shouldBe): (idxFrom): (inc): (idxEmptyAndSingle): * JSTests/stress/array-indexof-includes-result-boundaries.js: Added. (shouldBe): (i32Includes): (i32IncludesFrom): (dblIndex): (dblIncludes): (strIndex): (strIncludes): * Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp: * Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileArrayIndexOfOrArrayIncludes): Canonical link: https://commits.webkit.org/316534@main
1 parent 6115059 commit 23903f1

4 files changed

Lines changed: 224 additions & 58 deletions

File tree

‎JSTests/stress/array-indexof-includes-cow-atom-strings.js‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// indexOf/includes on copy-on-write "only atom strings" array literals, with a fallback
2+
// when the search element is not an atom.
3+
4+
function shouldBe(actual, expected) {
5+
if (actual !== expected)
6+
throw new Error(`bad value: expected ${expected} but got ${actual}`);
7+
}
8+
9+
function idx(s) { return ['__flags', '__methods', '_obj', 'assert'].indexOf(s); }
10+
noInline(idx);
11+
12+
function idxFrom(s, from) { return ['__flags', '__methods', '_obj', 'assert'].indexOf(s, from); }
13+
noInline(idxFrom);
14+
15+
function inc(s) { return ['__flags', '__methods', '_obj', 'assert'].includes(s); }
16+
noInline(inc);
17+
18+
function idxEmptyAndSingle(s) { return ['', 'a', 'bb'].indexOf(s); }
19+
noInline(idxEmptyAndSingle);
20+
21+
for (let i = 0; i < testLoopCount; ++i) {
22+
shouldBe(idx('__flags'), 0);
23+
shouldBe(idx('__methods'), 1);
24+
shouldBe(idx('_obj'), 2);
25+
shouldBe(idx('assert'), 3);
26+
27+
shouldBe(idx('nope'), -1);
28+
shouldBe(idx('then'), -1);
29+
30+
shouldBe(inc('__methods'), true);
31+
shouldBe(inc('xyz'), false);
32+
33+
// Non-atom search string equal to an element must hit the slow path and return the right index.
34+
const dyn = '_o' + 'bj';
35+
shouldBe(idx(dyn), 2);
36+
shouldBe(inc(dyn), true);
37+
shouldBe(idx('as' + 'sert'), 3);
38+
shouldBe(idx('no' + 'match'), -1);
39+
40+
shouldBe(idxFrom('__flags', 1), -1);
41+
shouldBe(idxFrom('assert', 2), 3);
42+
shouldBe(idxFrom('_obj', 3), -1);
43+
44+
shouldBe(idxEmptyAndSingle(''), 0);
45+
shouldBe(idxEmptyAndSingle('a'), 1);
46+
shouldBe(idxEmptyAndSingle('bb'), 2);
47+
shouldBe(idxEmptyAndSingle('' + ''), 0);
48+
shouldBe(idxEmptyAndSingle(String.fromCharCode(97)), 1);
49+
shouldBe(idxEmptyAndSingle('z'), -1);
50+
}

‎JSTests/stress/array-indexof-includes-result-boundaries.js‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// indexOf/includes result materialization across element types and tiers. Covers the boundary
2+
// cases for the branchless includes result (index != length): match at index 0, empty arrays
3+
// (length 0), and fromIndex >= length.
4+
5+
function shouldBe(actual, expected) {
6+
if (actual !== expected)
7+
throw new Error(`bad value: expected ${expected} but got ${actual}`);
8+
}
9+
10+
function i32Index(a, x) { return a.indexOf(x); }
11+
noInline(i32Index);
12+
function i32Includes(a, x) { return a.includes(x); }
13+
noInline(i32Includes);
14+
function i32IncludesFrom(a, x, from) { return a.includes(x, from); }
15+
noInline(i32IncludesFrom);
16+
17+
function dblIndex(a, x) { return a.indexOf(x); }
18+
noInline(dblIndex);
19+
function dblIncludes(a, x) { return a.includes(x); }
20+
noInline(dblIncludes);
21+
22+
function strIndex(a, x) { return a.indexOf(x); }
23+
noInline(strIndex);
24+
function strIncludes(a, x) { return a.includes(x); }
25+
noInline(strIncludes);
26+
27+
const i32 = [10, 20, 30, 40];
28+
const i32Empty = [];
29+
const dbl = [1.5, 2.5, 3.5];
30+
const dblEmpty = [];
31+
const str = ['a', 'bb', 'ccc'];
32+
const strEmpty = [];
33+
34+
for (let i = 0; i < testLoopCount; ++i) {
35+
// Match at index 0 must be found (guards against a naive "0 means not found" scheme).
36+
shouldBe(i32Index(i32, 10), 0);
37+
shouldBe(i32Includes(i32, 10), true);
38+
shouldBe(i32Index(i32, 40), 3);
39+
shouldBe(i32Includes(i32, 40), true);
40+
shouldBe(i32Index(i32, 99), -1);
41+
shouldBe(i32Includes(i32, 99), false);
42+
43+
shouldBe(dblIndex(dbl, 1.5), 0);
44+
shouldBe(dblIncludes(dbl, 1.5), true);
45+
shouldBe(dblIndex(dbl, 9.9), -1);
46+
shouldBe(dblIncludes(dbl, 9.9), false);
47+
48+
shouldBe(strIndex(str, 'a'), 0);
49+
shouldBe(strIncludes(str, 'a'), true);
50+
shouldBe(strIndex(str, 'zz'), -1);
51+
shouldBe(strIncludes(str, 'zz'), false);
52+
53+
// Empty arrays (length 0): includes -> false, indexOf -> -1.
54+
shouldBe(i32Index(i32Empty, 5), -1);
55+
shouldBe(i32Includes(i32Empty, 5), false);
56+
shouldBe(dblIndex(dblEmpty, 1.5), -1);
57+
shouldBe(dblIncludes(dblEmpty, 1.5), false);
58+
shouldBe(strIndex(strEmpty, 'x'), -1);
59+
shouldBe(strIncludes(strEmpty, 'x'), false);
60+
61+
// fromIndex >= length clamps to length, taking the not-found path.
62+
shouldBe(i32IncludesFrom(i32, 20, 5), false);
63+
shouldBe(i32IncludesFrom(i32, 10, 4), false);
64+
shouldBe(i32IncludesFrom(i32, 40, 3), true);
65+
}

‎Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp‎

Lines changed: 47 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -10328,6 +10328,20 @@ void SpeculativeJIT::compileArrayIndexOfOrArrayIncludes(Node* node)
1032810328
move(TrustedImm32(0), indexGPR);
1032910329

1033010330
Edge& searchElementEdge = m_graph.varArgChild(node, 1);
10331+
10332+
// Materialize the loop result into indexGPR by using lengthGPR comparison.
10333+
auto emitResult = [&](Jump notFound, JumpList found) {
10334+
if (isArrayIncludes) {
10335+
notFound.link(this);
10336+
found.link(this);
10337+
compare32(NotEqual, indexGPR, lengthGPR, indexGPR);
10338+
} else {
10339+
notFound.link(this);
10340+
move(TrustedImm32(-1), indexGPR);
10341+
found.link(this);
10342+
}
10343+
};
10344+
1033110345
switch (searchElementEdge.useKind()) {
1033210346
case Int32Use: {
1033310347
auto emitLoop = [&] (auto emitCompare) {
@@ -10346,20 +10360,11 @@ void SpeculativeJIT::compileArrayIndexOfOrArrayIncludes(Node* node)
1034610360
add32(TrustedImm32(1), indexGPR);
1034710361
jump().linkTo(loop, this);
1034810362

10349-
if (isArrayIncludes) {
10350-
notFound.link(this);
10351-
move(TrustedImm32(0), indexGPR);
10352-
Jump done = jump();
10353-
found.link(this);
10354-
move(TrustedImm32(1), indexGPR);
10355-
done.link(this);
10363+
emitResult(notFound, found);
10364+
if (isArrayIncludes)
1035610365
unblessedBooleanResult(indexGPR, node);
10357-
} else {
10358-
notFound.link(this);
10359-
move(TrustedImm32(-1), indexGPR);
10360-
found.link(this);
10366+
else
1036110367
strictInt32Result(indexGPR, node);
10362-
}
1036310368
};
1036410369

1036510370
ASSERT(node->arrayMode().type() == Array::Int32);
@@ -10411,20 +10416,11 @@ void SpeculativeJIT::compileArrayIndexOfOrArrayIncludes(Node* node)
1041110416
add32(TrustedImm32(1), indexGPR);
1041210417
jump().linkTo(loop, this);
1041310418

10414-
if (isArrayIncludes) {
10415-
notFound.link(this);
10416-
move(TrustedImm32(0), indexGPR);
10417-
Jump done = jump();
10418-
found.link(this);
10419-
move(TrustedImm32(1), indexGPR);
10420-
done.link(this);
10419+
emitResult(notFound, found);
10420+
if (isArrayIncludes)
1042110421
unblessedBooleanResult(indexGPR, node);
10422-
} else {
10423-
notFound.link(this);
10424-
move(TrustedImm32(-1), indexGPR);
10425-
found.link(this);
10422+
else
1042610423
strictInt32Result(indexGPR, node);
10427-
}
1042810424
return;
1042910425
}
1043010426

@@ -10485,22 +10481,33 @@ void SpeculativeJIT::compileArrayIndexOfOrArrayIncludes(Node* node)
1048510481
return false;
1048610482
};
1048710483

10484+
loadPtr(Address(searchElementGPR, JSString::offsetOfValue()), rightStringGPR);
10485+
if (canBeRope(searchElementEdge))
10486+
slowCase.append(branchIfRopeStringImpl(rightStringGPR));
10487+
10488+
Jump skipGeneric;
1048810489
if (isCopyOnWriteArrayWithContiguous()) {
1048910490
operation = operationCopyOnWriteArrayIndexOfString;
10490-
loadLinkableConstant(LinkableConstant(*this, vm().cellButterflyOnlyAtomStringsStructure.get()), compareLengthGPR);
10491-
emitEncodeStructureID(compareLengthGPR, compareLengthGPR);
10492-
addPtr(TrustedImm32(-static_cast<ptrdiff_t>(JSCellButterfly::offsetOfData())), storageGPR, leftStringGPR);
10493-
slowCase.append(branch32(Equal, Address(leftStringGPR, JSCell::structureIDOffset()), compareLengthGPR));
10491+
auto notAtomStructure = branchWeakStructure(NotEqual, Address(storageGPR, -static_cast<ptrdiff_t>(JSCellButterfly::offsetOfData()) + JSCell::structureIDOffset()), m_graph.registerStructure(vm().cellButterflyOnlyAtomStringsStructure.get()));
10492+
10493+
slowCase.append(branchTest32(Zero, Address(rightStringGPR, StringImpl::flagsOffset()), TrustedImm32(StringImpl::flagIsAtom())));
10494+
10495+
JumpList atomFound;
10496+
Label atomLoop = label();
10497+
Jump atomNotFound = branch32(Equal, indexGPR, lengthGPR);
10498+
loadPtr(BaseIndex(storageGPR, indexGPR, TimesEight), leftStringGPR);
10499+
loadPtr(Address(leftStringGPR, JSString::offsetOfValue()), leftStringGPR);
10500+
atomFound.append(branchPtr(Equal, leftStringGPR, rightStringGPR));
10501+
add32(TrustedImm32(1), indexGPR);
10502+
jump().linkTo(atomLoop, this);
10503+
10504+
emitResult(atomNotFound, atomFound);
10505+
skipGeneric = jump();
10506+
10507+
notAtomStructure.link(this);
1049410508
}
1049510509

10496-
loadPtr(Address(searchElementGPR, JSString::offsetOfValue()), rightStringGPR);
10497-
if (canBeRope(searchElementEdge))
10498-
slowCase.append(branchIfRopeStringImpl(rightStringGPR));
10499-
slowCase.append(branchTest32(
10500-
Zero,
10501-
Address(rightStringGPR, StringImpl::flagsOffset()),
10502-
TrustedImm32(StringImpl::flagIs8Bit())
10503-
));
10510+
slowCase.append(branchTest32(Zero, Address(rightStringGPR, StringImpl::flagsOffset()), TrustedImm32(StringImpl::flagIs8Bit())));
1050410511

1050510512
auto emitLoop = [&](auto emitCompare) {
1050610513
#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
@@ -10515,18 +10522,7 @@ void SpeculativeJIT::compileArrayIndexOfOrArrayIncludes(Node* node)
1051510522
add32(TrustedImm32(1), indexGPR);
1051610523
jump().linkTo(loop, this);
1051710524

10518-
if (isArrayIncludes) {
10519-
notFound.link(this);
10520-
move(TrustedImm32(0), indexGPR);
10521-
Jump done = jump();
10522-
found.link(this);
10523-
move(TrustedImm32(1), indexGPR);
10524-
done.link(this);
10525-
} else {
10526-
notFound.link(this);
10527-
move(TrustedImm32(-1), indexGPR);
10528-
found.link(this);
10529-
}
10525+
emitResult(notFound, found);
1053010526
};
1053110527

1053210528
auto emitCompare = [&]() -> JumpList {
@@ -10594,6 +10590,9 @@ void SpeculativeJIT::compileArrayIndexOfOrArrayIncludes(Node* node)
1059410590

1059510591
emitLoop(emitCompare);
1059610592

10593+
if (skipGeneric.isSet())
10594+
skipGeneric.link(this);
10595+
1059710596
if (isArrayIncludes) {
1059810597
addSlowPathGenerator(slowPathCall(
1059910598
slowCase, this, operationArrayIncludesString,

‎Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp‎

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8750,19 +8750,39 @@ IGNORE_CLANG_WARNINGS_END
87508750
return false;
87518751
};
87528752

8753-
if (isCopyOnWriteArrayWithContiguous()) {
8753+
bool isOnlyAtomStrings = isCopyOnWriteArrayWithContiguous();
8754+
LBasicBlock checkAtomStructure = nullptr;
8755+
LBasicBlock atomCheckSearchIsAtom = nullptr;
8756+
LBasicBlock atomLoopHeader = nullptr;
8757+
LBasicBlock atomLoopBody = nullptr;
8758+
LBasicBlock atomLoopNext = nullptr;
8759+
LBasicBlock atomNotFound = nullptr;
8760+
ValueFromBlock initialStartIndexForAtom;
8761+
8762+
if (isOnlyAtomStrings) {
87548763
operation = operationCopyOnWriteArrayIndexOfString;
8755-
LValue targetStructureID = encodeStructureID(weakPointer(vm().cellButterflyOnlyAtomStringsStructure.get()));
8756-
LValue butterflyStructureID = m_out.load32(m_out.add(storage, m_out.constIntPtr(-JSCellButterfly::offsetOfData())), m_heaps.JSCell_structureID);
8757-
m_out.branch(m_out.equal(butterflyStructureID, targetStructureID), unsure(slowCase), unsure(checkSearchRopeString));
8758-
} else
8759-
m_out.jump(checkSearchRopeString);
8764+
checkAtomStructure = m_out.newBlock();
8765+
atomCheckSearchIsAtom = m_out.newBlock();
8766+
atomLoopHeader = m_out.newBlock();
8767+
atomLoopBody = m_out.newBlock();
8768+
atomLoopNext = m_out.newBlock();
8769+
atomNotFound = m_out.newBlock();
8770+
initialStartIndexForAtom = m_out.anchor(startIndex);
8771+
}
8772+
m_out.jump(checkSearchRopeString);
87608773

87618774
LBasicBlock lastNext = m_out.appendTo(checkSearchRopeString, checkSearchElement8Bit);
8762-
m_out.branch(isRopeString(searchElement, searchElementEdge), rarely(slowCase), usually(checkSearchElement8Bit));
8775+
LValue searchElementImpl = m_out.loadPtr(searchElement, m_heaps.JSString_value);
8776+
m_out.branch(isRopeString(searchElement, searchElementEdge), rarely(slowCase), usually(isOnlyAtomStrings ? checkAtomStructure : checkSearchElement8Bit));
8777+
8778+
if (isOnlyAtomStrings) {
8779+
m_out.appendTo(checkAtomStructure, checkSearchElement8Bit);
8780+
LValue targetStructureID = weakStructureID(m_graph.registerStructure(vm().cellButterflyOnlyAtomStringsStructure.get()));
8781+
LValue butterflyStructureID = m_out.load32(m_out.add(storage, m_out.constIntPtr(-JSCellButterfly::offsetOfData())), m_heaps.JSCell_structureID);
8782+
m_out.branch(m_out.equal(butterflyStructureID, targetStructureID), unsure(atomCheckSearchIsAtom), unsure(checkSearchElement8Bit));
8783+
}
87638784

87648785
m_out.appendTo(checkSearchElement8Bit, loopHeader);
8765-
LValue searchElementImpl = m_out.loadPtr(searchElement, m_heaps.JSString_value);
87668786
m_out.branch(m_out.testIsZero32(m_out.load32(searchElementImpl, m_heaps.StringImpl_hashAndFlags), m_out.constInt32(StringImpl::flagIs8Bit())), unsure(slowCase), unsure(loopHeader));
87678787

87688788
m_out.appendTo(loopHeader, fastCheckElementEmpty);
@@ -8859,13 +8879,45 @@ IGNORE_CLANG_WARNINGS_END
88598879
ValueFromBlock slowCaseResult = isArrayIncludes ? m_out.anchor(vmCall(Int32, operationArrayIncludesString, weakPointer(globalObject), storage, searchElement, startIndex)) : m_out.anchor(vmCall(Int64, operation, weakPointer(globalObject), storage, searchElement, startIndex));
88608880
m_out.jump(continuation);
88618881

8882+
Vector<ValueFromBlock, 5> results;
8883+
results.append(notFoundResult);
8884+
results.append(foundResult);
8885+
results.append(slowCaseResult);
8886+
8887+
if (isOnlyAtomStrings) {
8888+
m_out.appendTo(atomCheckSearchIsAtom, atomLoopHeader);
8889+
m_out.branch(m_out.testIsZero32(m_out.load32(searchElementImpl, m_heaps.StringImpl_hashAndFlags), m_out.constInt32(StringImpl::flagIsAtom())), rarely(slowCase), usually(atomLoopHeader));
8890+
8891+
m_out.appendTo(atomLoopHeader, atomLoopBody);
8892+
LValue atomIndex = m_out.phi(pointerType(), initialStartIndexForAtom);
8893+
m_out.branch(m_out.notEqual(atomIndex, length), usually(atomLoopBody), rarely(atomNotFound));
8894+
8895+
m_out.appendTo(atomLoopBody, atomLoopNext);
8896+
ValueFromBlock atomFoundResult = isArrayIncludes ? m_out.anchor(m_out.constBool(true)) : m_out.anchor(atomIndex);
8897+
LValue atomElement = m_out.load64(m_out.baseIndex(m_heaps.indexedContiguousProperties, storage, atomIndex));
8898+
LValue atomElementImpl = m_out.loadPtr(atomElement, m_heaps.JSString_value);
8899+
m_out.branch(m_out.equal(atomElementImpl, searchElementImpl), rarely(continuation), usually(atomLoopNext));
8900+
8901+
m_out.appendTo(atomLoopNext, atomNotFound);
8902+
LValue atomNextIndex = m_out.add(atomIndex, m_out.intPtrOne);
8903+
m_out.addIncomingToPhi(atomIndex, m_out.anchor(atomNextIndex));
8904+
m_out.jump(atomLoopHeader);
8905+
8906+
m_out.appendTo(atomNotFound, continuation);
8907+
ValueFromBlock atomNotFoundResult = isArrayIncludes ? m_out.anchor(m_out.constBool(false)) : m_out.anchor(m_out.constIntPtr(-1));
8908+
m_out.jump(continuation);
8909+
8910+
results.append(atomNotFoundResult);
8911+
results.append(atomFoundResult);
8912+
}
8913+
88628914
m_out.appendTo(continuation, lastNext);
88638915
// We have to keep base alive since that keeps content of storage alive.
88648916
ensureStillAliveHere(base);
88658917
if (isArrayIncludes)
8866-
setBoolean(m_out.phi(Int32, notFoundResult, foundResult, slowCaseResult));
8918+
setBoolean(m_out.phi(Int32, results));
88678919
else
8868-
setInt32(m_out.castToInt32(m_out.phi(Int64, notFoundResult, foundResult, slowCaseResult)));
8920+
setInt32(m_out.castToInt32(m_out.phi(Int64, results)));
88698921
return;
88708922
}
88718923

0 commit comments

Comments
 (0)