Skip to content

Commit 62300f8

Browse files
ConstellationConstellation
committed
[JSC] Immutable Load should look for the CSE target in dominators
https://bugs.webkit.org/show_bug.cgi?id=299504 rdar://161294228 Reviewed by Yijia Huang. Previous one 300327@main worked for the same basic block's load, but it didn't work for the load in dominators. This patch updates CSE rules to make immutable load elimination work with dominators' load. Like, BB#0 @0: Load(@x, immutable) @1: CCall(...) # potentially clobber everything Branch ... #1, #2 BB#1 @2: CCall(...) # potentially clobber everything Jump #3 BB#2 @3: CCall(...) # potentially clobber everything Jump #3 BB#3 @4: Load(@x, immutable) ... Then @4 should be replaced with Identity(@0) as dominator BB#0 is having immutable load @0 matching to @4. Tests: Source/JavaScriptCore/b3/testb3_1.cpp Source/JavaScriptCore/b3/testb3_8.cpp * Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp: * Source/JavaScriptCore/b3/testb3.h: * Source/JavaScriptCore/b3/testb3_1.cpp: (run): * Source/JavaScriptCore/b3/testb3_8.cpp: (testLoadImmutableDominated): (testLoadImmutableNonDominated): Canonical link: https://commits.webkit.org/300562@main
1 parent 3c71d5e commit 62300f8

File tree

4 files changed

+81
-2
lines changed

4 files changed

+81
-2
lines changed

‎Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,7 @@ class CSE {
697697
return { match };
698698
}
699699

700-
if (m_data.writes.overlaps(range)) {
700+
if (m_data.writes.overlaps(range) && m_value->effects().readsMutability != Mutability::Immutable) {
701701
dataLogLnIf(B3EliminateCommonSubexpressionsInternal::verbose, " Giving up because of writes.");
702702
return { };
703703
}
@@ -720,7 +720,7 @@ class CSE {
720720
continue;
721721
}
722722

723-
if (data.writes.overlaps(range)) {
723+
if (data.writes.overlaps(range) && m_value->effects().readsMutability != Mutability::Immutable) {
724724
dataLogLnIf(B3EliminateCommonSubexpressionsInternal::verbose, " Giving up because of writes.");
725725
return { };
726726
}

‎Source/JavaScriptCore/b3/testb3.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,5 +1401,7 @@ void testMemoryFill();
14011401
void testMemoryFillConstant();
14021402

14031403
void testLoadImmutable();
1404+
void testLoadImmutableDominated();
1405+
void testLoadImmutableNonDominated();
14041406

14051407
#endif // ENABLE(B3_JIT)

‎Source/JavaScriptCore/b3/testb3_1.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -895,6 +895,8 @@ void run(const TestConfig* config)
895895
RUN(testConstFloatMove());
896896

897897
RUN(testLoadImmutable());
898+
RUN(testLoadImmutableDominated());
899+
RUN(testLoadImmutableNonDominated());
898900

899901
RUN_UNARY(testSShrCompare32, int32OperandsMore());
900902
RUN_UNARY(testSShrCompare64, int64OperandsMore());

‎Source/JavaScriptCore/b3/testb3_8.cpp‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1851,6 +1851,81 @@ void testLoadImmutable()
18511851
CHECK_EQ(invoke<uint64_t>(*code, memory.mutableSpan().data(), memory.mutableSpan().data() + 1), 84U);
18521852
}
18531853

1854+
void testLoadImmutableDominated()
1855+
{
1856+
Vector<uint64_t> memory(4);
1857+
Procedure proc;
1858+
BasicBlock* root = proc.addBlock();
1859+
BasicBlock* thenCase = proc.addBlock();
1860+
BasicBlock* elseCase = proc.addBlock();
1861+
BasicBlock* done = proc.addBlock();
1862+
auto arguments = cCallArgumentValues<void*, void*>(proc, root);
1863+
1864+
auto* value1 = root->appendNew<MemoryValue>(proc, Load, Int64, Origin(), arguments[0]);
1865+
value1->setReadsMutability(B3::Mutability::Immutable);
1866+
root->appendNew<MemoryValue>(proc, Store, Origin(), root->appendNew<Const32Value>(proc, Origin(), 0), arguments[1]);
1867+
root->appendNew<Value>(
1868+
proc, Branch, Origin(),
1869+
root->appendNew<Value>(proc, B3::Equal, Origin(), value1, root->appendIntConstant(proc, Origin(), Int64, 42)));
1870+
root->setSuccessors(thenCase, elseCase);
1871+
1872+
thenCase->appendNew<MemoryValue>(proc, Store, Origin(), thenCase->appendNew<Const32Value>(proc, Origin(), 22), arguments[1]);
1873+
thenCase->appendNew<Value>(proc, Jump, Origin());
1874+
thenCase->setSuccessors(done);
1875+
1876+
elseCase->appendNew<MemoryValue>(proc, Store, Origin(), elseCase->appendNew<Const32Value>(proc, Origin(), 11), arguments[1]);
1877+
elseCase->appendNew<Value>(proc, Jump, Origin());
1878+
elseCase->setSuccessors(done);
1879+
1880+
auto* value2 = done->appendNew<MemoryValue>(proc, Load, Int64, Origin(), arguments[0]);
1881+
value2->setReadsMutability(B3::Mutability::Immutable);
1882+
done->appendNewControlValue(proc, Return, Origin(), done->appendNew<Value>(proc, Add, Origin(), value1, value2));
1883+
auto code = compileProc(proc);
1884+
1885+
memory.fill(42);
1886+
CHECK_EQ(invoke<uint64_t>(*code, memory.mutableSpan().data(), memory.mutableSpan().data() + 1), 84U);
1887+
memory.fill(11);
1888+
CHECK_EQ(invoke<uint64_t>(*code, memory.mutableSpan().data(), memory.mutableSpan().data() + 1), 22U);
1889+
}
1890+
1891+
void testLoadImmutableNonDominated()
1892+
{
1893+
Vector<uint64_t> memory(4);
1894+
Procedure proc;
1895+
BasicBlock* root = proc.addBlock();
1896+
BasicBlock* thenCase = proc.addBlock();
1897+
BasicBlock* elseCase = proc.addBlock();
1898+
BasicBlock* done = proc.addBlock();
1899+
auto arguments = cCallArgumentValues<void*, void*>(proc, root);
1900+
1901+
auto* cond = thenCase->appendNew<MemoryValue>(proc, Load, Int64, Origin(), arguments[1]);
1902+
root->appendNew<MemoryValue>(proc, Store, Origin(), root->appendNew<Const32Value>(proc, Origin(), 0), arguments[1]);
1903+
root->appendNew<Value>(
1904+
proc, Branch, Origin(),
1905+
root->appendNew<Value>(proc, B3::Equal, Origin(), cond, root->appendIntConstant(proc, Origin(), Int64, 42)));
1906+
root->setSuccessors(thenCase, elseCase);
1907+
1908+
auto* value1 = thenCase->appendNew<MemoryValue>(proc, Load, Int64, Origin(), arguments[0]);
1909+
value1->setReadsMutability(B3::Mutability::Immutable);
1910+
thenCase->appendNew<MemoryValue>(proc, Store, Origin(), value1, arguments[1]);
1911+
thenCase->appendNew<Value>(proc, Jump, Origin());
1912+
thenCase->setSuccessors(done);
1913+
1914+
elseCase->appendNew<MemoryValue>(proc, Store, Origin(), elseCase->appendNew<Const32Value>(proc, Origin(), 11), arguments[1]);
1915+
elseCase->appendNew<Value>(proc, Jump, Origin());
1916+
elseCase->setSuccessors(done);
1917+
1918+
auto* value2 = done->appendNew<MemoryValue>(proc, Load, Int64, Origin(), arguments[0]);
1919+
value2->setReadsMutability(B3::Mutability::Immutable);
1920+
done->appendNewControlValue(proc, Return, Origin(), value2);
1921+
auto code = compileProc(proc);
1922+
1923+
memory.fill(42);
1924+
CHECK_EQ(invoke<uint64_t>(*code, memory.mutableSpan().data(), memory.mutableSpan().data() + 1), 42U);
1925+
memory.fill(11);
1926+
CHECK_EQ(invoke<uint64_t>(*code, memory.mutableSpan().data(), memory.mutableSpan().data() + 1), 11U);
1927+
}
1928+
18541929
#endif // ENABLE(B3_JIT)
18551930

18561931
WTF_ALLOW_UNSAFE_BUFFER_USAGE_END

0 commit comments

Comments
 (0)