Log Message
Cherry-pick r292475. rdar://problem/91078546
CSE should be more careful with values that have WritesPinned, ExitsSideways, or are of different sizes https://bugs.webkit.org/show_bug.cgi?id=238302 Reviewed by Saam Barati. 1) CSE for WasmAddressValue searches for any nodes marked with WritesPinned between all paths from the redundant WasmAddressValue to the replacement that dominates it. For a switch construct, we might miss some paths while performing this DFS of predecessor blocks because the termination condition was a break instead of a continue. 2) The CSE phase looks for a pattern it calls a store after clobber. That is, two stores to the same location where the first store is not observable. It searches for reads and writes that overlap with the store and its clobber, and bails if it finds them. When we add in CSE for WasmAddressValue, we expose the fact that WasmBoundsCheck has ExitSideways but does not claim to read top, even though the code it exits to totally can. This can cause us to eliminate an observable store. 3) The store after clobber phase does not check that the size of the clobber is the same as the size of the store. Again, this is usually hidden by _javascript_ because the sizes are usually the same. Also, when WASM fast memory is enabled, this bug is hidden because the loads/stores say that they may trap. That is why the unity test case only failed on iOS (although it does fail on mac with __XPC_JSC_useWebAssemblyFastMemory=0). * b3/B3EliminateCommonSubexpressions.cpp: * b3/testb3.h: * b3/testb3_8.cpp: (testWasmAddressDoesNotCSE): (addCopyTests): git-svn-id: https://svn.webkit.org/repository/webkit/trunk@292475 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
- branches/safari-613-branch/Source/_javascript_Core/ChangeLog
- branches/safari-613-branch/Source/_javascript_Core/b3/B3EliminateCommonSubexpressions.cpp
- branches/safari-613-branch/Source/_javascript_Core/b3/B3Validate.cpp
- branches/safari-613-branch/Source/_javascript_Core/b3/B3Value.cpp
- branches/safari-613-branch/Source/_javascript_Core/b3/testb3.h
- branches/safari-613-branch/Source/_javascript_Core/b3/testb3_1.cpp
- branches/safari-613-branch/Source/_javascript_Core/b3/testb3_7.cpp
- branches/safari-613-branch/Source/_javascript_Core/b3/testb3_8.cpp
- branches/safari-613-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp
Diff
Modified: branches/safari-613-branch/Source/_javascript_Core/ChangeLog (292989 => 292990)
--- branches/safari-613-branch/Source/_javascript_Core/ChangeLog 2022-04-19 00:49:38 UTC (rev 292989)
+++ branches/safari-613-branch/Source/_javascript_Core/ChangeLog 2022-04-19 00:49:42 UTC (rev 292990)
@@ -1,5 +1,64 @@
2022-04-18 Kocsen Chung <kocsen_ch...@apple.com>
+ Cherry-pick r292475. rdar://problem/91078546
+
+ CSE should be more careful with values that have WritesPinned, ExitsSideways, or are of different sizes
+ https://bugs.webkit.org/show_bug.cgi?id=238302
+
+ Reviewed by Saam Barati.
+
+ 1) CSE for WasmAddressValue searches for any nodes marked with WritesPinned between all paths from the redundant
+ WasmAddressValue to the replacement that dominates it. For a switch construct, we might miss some paths while
+ performing this DFS of predecessor blocks because the termination condition was a break instead of a continue.
+
+ 2) The CSE phase looks for a pattern it calls a store after clobber. That is, two stores to the same location where
+ the first store is not observable. It searches for reads and writes that overlap with the store and its clobber, and
+ bails if it finds them. When we add in CSE for WasmAddressValue, we expose the fact that WasmBoundsCheck has ExitSideways
+ but does not claim to read top, even though the code it exits to totally can. This can cause us to eliminate an observable store.
+
+ 3) The store after clobber phase does not check that the size of the clobber is the same as the size of the store. Again,
+ this is usually hidden by _javascript_ because the sizes are usually the same. Also, when WASM fast memory is enabled, this
+ bug is hidden because the loads/stores say that they may trap. That is why the unity test case only failed on iOS (although
+ it does fail on mac with __XPC_JSC_useWebAssemblyFastMemory=0).
+
+ * b3/B3EliminateCommonSubexpressions.cpp:
+ * b3/testb3.h:
+ * b3/testb3_8.cpp:
+ (testWasmAddressDoesNotCSE):
+ (addCopyTests):
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@292475 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-04-06 Justin Michaud <justin_mich...@apple.com>
+
+ CSE should be more careful with values that have WritesPinned, ExitsSideways, or are of different sizes
+ https://bugs.webkit.org/show_bug.cgi?id=238302
+
+ Reviewed by Saam Barati.
+
+ 1) CSE for WasmAddressValue searches for any nodes marked with WritesPinned between all paths from the redundant
+ WasmAddressValue to the replacement that dominates it. For a switch construct, we might miss some paths while
+ performing this DFS of predecessor blocks because the termination condition was a break instead of a continue.
+
+ 2) The CSE phase looks for a pattern it calls a store after clobber. That is, two stores to the same location where
+ the first store is not observable. It searches for reads and writes that overlap with the store and its clobber, and
+ bails if it finds them. When we add in CSE for WasmAddressValue, we expose the fact that WasmBoundsCheck has ExitSideways
+ but does not claim to read top, even though the code it exits to totally can. This can cause us to eliminate an observable store.
+
+ 3) The store after clobber phase does not check that the size of the clobber is the same as the size of the store. Again,
+ this is usually hidden by _javascript_ because the sizes are usually the same. Also, when WASM fast memory is enabled, this
+ bug is hidden because the loads/stores say that they may trap. That is why the unity test case only failed on iOS (although
+ it does fail on mac with __XPC_JSC_useWebAssemblyFastMemory=0).
+
+ * b3/B3EliminateCommonSubexpressions.cpp:
+ * b3/testb3.h:
+ * b3/testb3_8.cpp:
+ (testWasmAddressDoesNotCSE):
+ (addCopyTests):
+
+2022-04-18 Kocsen Chung <kocsen_ch...@apple.com>
+
Cherry-pick r292269. rdar://problem/91204413
AI should do int32 optimization in ValueRep
Modified: branches/safari-613-branch/Source/_javascript_Core/b3/B3EliminateCommonSubexpressions.cpp (292989 => 292990)
--- branches/safari-613-branch/Source/_javascript_Core/b3/B3EliminateCommonSubexpressions.cpp 2022-04-19 00:49:38 UTC (rev 292989)
+++ branches/safari-613-branch/Source/_javascript_Core/b3/B3EliminateCommonSubexpressions.cpp 2022-04-19 00:49:42 UTC (rev 292990)
@@ -33,6 +33,7 @@
#include "B3HeapRange.h"
#include "B3InsertionSetInlines.h"
#include "B3MemoryValue.h"
+#include "B3MemoryValueInlines.h"
#include "B3PhaseScope.h"
#include "B3ProcedureInlines.h"
#include "B3PureCSE.h"
@@ -249,6 +250,7 @@
m_value->performSubstitution();
if (m_pureCSE.process(m_value, m_dominators)) {
+ ASSERT(!m_value->effects().readsPinned || !m_data.writesPinned);
ASSERT(!m_value->effects().writes);
ASSERT(!m_value->effects().writesPinned);
m_changed = true;
@@ -477,11 +479,13 @@
}
case Store: {
+ auto clobberWidth = memory->accessWidth();
handleStoreAfterClobber(
ptr, range,
[&] (MemoryValue* candidate) -> bool {
return candidate->opcode() == Store
- && candidate->offset() == offset;
+ && candidate->offset() == offset
+ && candidate->accessWidth() >= clobberWidth;
});
break;
}
@@ -725,6 +729,8 @@
Value* ptr = wasmAddress->child(0);
if (Value* replacement = m_data.m_candidateWasmAddressesAtTail.get(ptr)) {
+ if (B3EliminateCommonSubexpressionsInternal::verbose)
+ dataLog(" Replacing WasmAddress: ", *wasmAddress, " with ", *replacement, "\n");
wasmAddress->replaceWithIdentity(replacement);
m_changed = true;
return;
@@ -759,7 +765,7 @@
worklist.pushAll(m_block->predecessors());
while (BasicBlock* block = worklist.pop()) {
if (block == dominator)
- break;
+ continue;
if (m_impureBlockData[block].writesPinned) {
candidateReplacement = nullptr;
break;
@@ -768,6 +774,8 @@
}
if (candidateReplacement) {
+ if (B3EliminateCommonSubexpressionsInternal::verbose)
+ dataLog(" Replacing WasmAddress: ", *wasmAddress, " with ", *candidateReplacement, "\n");
wasmAddress->replaceWithIdentity(candidateReplacement);
m_changed = true;
}
Modified: branches/safari-613-branch/Source/_javascript_Core/b3/B3Validate.cpp (292989 => 292990)
--- branches/safari-613-branch/Source/_javascript_Core/b3/B3Validate.cpp 2022-04-19 00:49:38 UTC (rev 292989)
+++ branches/safari-613-branch/Source/_javascript_Core/b3/B3Validate.cpp 2022-04-19 00:49:42 UTC (rev 292990)
@@ -25,6 +25,7 @@
#include "config.h"
#include "B3Validate.h"
+#include "B3HeapRange.h"
#if ENABLE(B3_JIT)
@@ -567,6 +568,10 @@
}
VALIDATE(!(value->effects().writes && value->key()), ("At ", *value));
+ // Conceptually, if you exit sideways, you will almost certainly read Top or (AbstractHeapRepository) root.
+ // Since we can't easily find out what root is here, we settle for checking that you at least read *something*.
+ // This is technically overly strict, but in practice violating this is almost certainly a bug.
+ VALIDATE((!value->effects().exitsSideways || value->effects().reads != HeapRange()), ("At ", *value));
}
for (Variable* variable : m_procedure.variables())
Modified: branches/safari-613-branch/Source/_javascript_Core/b3/B3Value.cpp (292989 => 292990)
--- branches/safari-613-branch/Source/_javascript_Core/b3/B3Value.cpp 2022-04-19 00:49:38 UTC (rev 292989)
+++ branches/safari-613-branch/Source/_javascript_Core/b3/B3Value.cpp 2022-04-19 00:49:42 UTC (rev 292990)
@@ -669,6 +669,7 @@
break;
}
result.exitsSideways = true;
+ result.reads = HeapRange::top();
break;
case Upsilon:
case Set:
Modified: branches/safari-613-branch/Source/_javascript_Core/b3/testb3.h (292989 => 292990)
--- branches/safari-613-branch/Source/_javascript_Core/b3/testb3.h 2022-04-19 00:49:38 UTC (rev 292989)
+++ branches/safari-613-branch/Source/_javascript_Core/b3/testb3.h 2022-04-19 00:49:42 UTC (rev 292990)
@@ -1176,4 +1176,10 @@
void testFloatMaxMin();
void testDoubleMaxMin();
+void testWasmAddressDoesNotCSE();
+void testStoreAfterClobberExitsSideways();
+void testStoreAfterClobberDifferentWidth();
+void testStoreAfterClobberDifferentWidthSuccessor();
+void testStoreAfterClobberExitsSidewaysSuccessor();
+
#endif // ENABLE(B3_JIT)
Modified: branches/safari-613-branch/Source/_javascript_Core/b3/testb3_1.cpp (292989 => 292990)
--- branches/safari-613-branch/Source/_javascript_Core/b3/testb3_1.cpp 2022-04-19 00:49:38 UTC (rev 292989)
+++ branches/safari-613-branch/Source/_javascript_Core/b3/testb3_1.cpp 2022-04-19 00:49:42 UTC (rev 292990)
@@ -541,6 +541,12 @@
RUN(testSpillGP());
RUN(testSpillFP());
+ RUN(testWasmAddressDoesNotCSE());
+ RUN(testStoreAfterClobberDifferentWidth());
+ RUN(testStoreAfterClobberExitsSideways());
+ RUN(testStoreAfterClobberDifferentWidthSuccessor());
+ RUN(testStoreAfterClobberExitsSidewaysSuccessor());
+
RUN(testInt32ToDoublePartialRegisterStall());
RUN(testInt32ToDoublePartialRegisterWithoutStall());
Modified: branches/safari-613-branch/Source/_javascript_Core/b3/testb3_7.cpp (292989 => 292990)
--- branches/safari-613-branch/Source/_javascript_Core/b3/testb3_7.cpp 2022-04-19 00:49:38 UTC (rev 292989)
+++ branches/safari-613-branch/Source/_javascript_Core/b3/testb3_7.cpp 2022-04-19 00:49:42 UTC (rev 292990)
@@ -596,6 +596,7 @@
[&] (BasicBlock* loop, Value*) -> Value* {
Effects effects = Effects::none();
effects.exitsSideways = true;
+ effects.reads = HeapRange::top();
loop->appendNew<CCallValue>(
proc, Void, Origin(), effects,
loop->appendNew<ConstPtrValue>(proc, Origin(), tagCFunction<OperationPtrTag>(noOpFunction)));
@@ -789,6 +790,7 @@
[&] (BasicBlock* loop, Value*) -> Value* {
Effects effects = Effects::none();
effects.exitsSideways = true;
+ effects.reads = HeapRange::top();
return loop->appendNew<CCallValue>(
proc, Int32, Origin(), effects,
loop->appendNew<ConstPtrValue>(proc, Origin(), tagCFunction<OperationPtrTag>(oneFunction)),
@@ -927,6 +929,7 @@
[&] (BasicBlock* loop, Value*) -> Value* {
Effects effects = Effects::none();
effects.exitsSideways = true;
+ effects.reads = HeapRange::top();
loop->appendNew<CCallValue>(
proc, Void, Origin(), effects,
loop->appendNew<ConstPtrValue>(proc, Origin(), tagCFunction<OperationPtrTag>(noOpFunction)));
Modified: branches/safari-613-branch/Source/_javascript_Core/b3/testb3_8.cpp (292989 => 292990)
--- branches/safari-613-branch/Source/_javascript_Core/b3/testb3_8.cpp 2022-04-19 00:49:38 UTC (rev 292989)
+++ branches/safari-613-branch/Source/_javascript_Core/b3/testb3_8.cpp 2022-04-19 00:49:42 UTC (rev 292990)
@@ -1162,6 +1162,424 @@
delete [] arr2;
}
+void testWasmAddressDoesNotCSE()
+{
+ Procedure proc;
+ GPRReg pinnedGPR = GPRInfo::argumentGPR0;
+ proc.pinRegister(pinnedGPR);
+
+ BasicBlock* root = proc.addBlock();
+ BasicBlock* a = proc.addBlock();
+ BasicBlock* b = proc.addBlock();
+ BasicBlock* c = proc.addBlock();
+ BasicBlock* continuation = proc.addBlock();
+
+ auto* pointer = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
+ auto* path = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR2);
+
+ auto* originalAddress = root->appendNew<WasmAddressValue>(proc, Origin(), pointer, pinnedGPR);
+ root->appendNew<MemoryValue>(proc, Store, Origin(), originalAddress,
+ root->appendNew<WasmAddressValue>(proc, Origin(), root->appendNew<Const64Value>(proc, Origin(), 6*8), pinnedGPR), 0);
+
+ SwitchValue* switchValue = root->appendNew<SwitchValue>(proc, Origin(), path);
+ switchValue->setFallThrough(FrequentedBlock(c));
+ switchValue->appendCase(SwitchCase(0, FrequentedBlock(a)));
+ switchValue->appendCase(SwitchCase(1, FrequentedBlock(b)));
+
+ PatchpointValue* patchpoint = b->appendNew<PatchpointValue>(proc, Void, Origin());
+ patchpoint->effects = Effects::forCall();
+ patchpoint->clobber(RegisterSet::macroScratchRegisters());
+ patchpoint->clobber(RegisterSet(pinnedGPR));
+ patchpoint->setGenerator(
+ [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+ CHECK(!params.size());
+ jit.add64(MacroAssembler::TrustedImm32(8), pinnedGPR);
+ });
+
+ UpsilonValue* takeA = a->appendNew<UpsilonValue>(proc, Origin(), a->appendNew<Const32Value>(proc, Origin(), 10));
+ UpsilonValue* takeB = b->appendNew<UpsilonValue>(proc, Origin(), b->appendNew<Const32Value>(proc, Origin(), 20));
+ UpsilonValue* takeC = c->appendNew<UpsilonValue>(proc, Origin(), c->appendNew<Const32Value>(proc, Origin(), 30));
+ for (auto* i : { a, b, c }) {
+ i->appendNewControlValue(proc, Jump, Origin(), FrequentedBlock(continuation));
+ i->setSuccessors(FrequentedBlock(continuation));
+ }
+
+ // Continuation
+ auto* takenPhi = continuation->appendNew<Value>(proc, Phi, Int32, Origin());
+
+ auto* address2 = continuation->appendNew<WasmAddressValue>(proc, Origin(), pointer, pinnedGPR);
+ continuation->appendNew<MemoryValue>(proc, Store, Origin(), takenPhi,
+ continuation->appendNew<WasmAddressValue>(proc, Origin(), continuation->appendNew<Const64Value>(proc, Origin(), 4*8), pinnedGPR),
+ 0);
+
+ auto* returnVal = address2;
+ continuation->appendNewControlValue(proc, Return, Origin(), returnVal);
+
+ takeA->setPhi(takenPhi);
+ takeB->setPhi(takenPhi);
+ takeC->setPhi(takenPhi);
+
+ auto binary = compileProc(proc);
+
+ uint64_t* memory = new uint64_t[10];
+ uint64_t ptr = 8;
+
+ uint64_t finalPtr = reinterpret_cast<uint64_t>(static_cast<void*>(memory)) + ptr;
+
+ for (int i = 0; i < 10; ++i)
+ memory[i] = 0;
+
+ {
+ uint64_t result = invoke<uint64_t>(*binary, memory, ptr, 0);
+
+ CHECK_EQ(result, finalPtr);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], 0ul);
+ CHECK_EQ(memory[2], 0ul);
+ CHECK_EQ(memory[4], 10ul);
+ CHECK_EQ(memory[6], finalPtr);
+ }
+
+ memory[4] = 0;
+ memory[5] = 0;
+ memory[6] = 0;
+ memory[7] = 0;
+
+ {
+ uint64_t result = invoke<uint64_t>(*binary, memory, ptr, 1);
+
+ CHECK_EQ(result, finalPtr + 8);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], 0ul);
+ CHECK_EQ(memory[2], 0ul);
+ CHECK_EQ(memory[5], 20ul);
+ CHECK_EQ(memory[6], finalPtr);
+ }
+
+ memory[4] = 0;
+ memory[5] = 0;
+ memory[6] = 0;
+ memory[7] = 0;
+ {
+ uint64_t result = invoke<uint64_t>(*binary, memory, ptr, 2);
+
+ CHECK_EQ(result, finalPtr);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], 0ul);
+ CHECK_EQ(memory[2], 0ul);
+ CHECK_EQ(memory[4], 30ul);
+ CHECK_EQ(memory[6], finalPtr);
+ }
+
+ delete[] memory;
+}
+
+void testStoreAfterClobberExitsSideways()
+{
+ Procedure proc;
+ GPRReg pinnedBaseGPR = GPRInfo::argumentGPR0;
+ GPRReg pinnedSizeGPR = GPRInfo::argumentGPR1;
+ proc.pinRegister(pinnedBaseGPR);
+ proc.pinRegister(pinnedSizeGPR);
+
+ // Please don't make me save anything.
+ RegisterSet csrs;
+ csrs.merge(RegisterSet::calleeSaveRegisters());
+ csrs.exclude(RegisterSet::stackRegisters());
+ csrs.forEach(
+ [&] (Reg reg) {
+ CHECK(reg != pinnedBaseGPR);
+ CHECK(reg != pinnedSizeGPR);
+ proc.pinRegister(reg);
+ });
+
+ proc.setWasmBoundsCheckGenerator([=] (CCallHelpers& jit, GPRReg pinnedGPR) {
+ CHECK_EQ(pinnedGPR, pinnedSizeGPR);
+
+ jit.move(CCallHelpers::TrustedImm32(42), GPRInfo::returnValueGPR);
+ jit.emitFunctionEpilogue();
+ jit.ret();
+ });
+
+ BasicBlock* root = proc.addBlock();
+
+ auto* pointer = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR2);
+ auto* resultAddress = root->appendNew<WasmAddressValue>(proc, Origin(), pointer, pinnedBaseGPR);
+ root->appendNew<MemoryValue>(proc, Store, Origin(), root->appendNew<Const32Value>(proc, Origin(), 10), resultAddress, 0);
+
+ root->appendNew<WasmBoundsCheckValue>(proc, Origin(), pinnedSizeGPR, root->appendNew<Value>(proc, Trunc, Origin(), pointer), 0);
+
+ root->appendNew<MemoryValue>(proc, Store, Origin(), root->appendNew<Const32Value>(proc, Origin(), 20), resultAddress, 0);
+ root->appendNewControlValue(proc, Return, Origin(), root->appendNew<Const32Value>(proc, Origin(), 30));
+
+ auto binary = compileProc(proc);
+
+ uint64_t* memory = new uint64_t[10];
+ uint64_t ptr = 1*8;
+
+ for (int i = 0; i < 10; ++i)
+ memory[i] = 0;
+
+ {
+ int result = invoke<int>(*binary, memory, 16, ptr);
+
+ CHECK_EQ(result, 30);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], 20ul);
+ CHECK_EQ(memory[2], 0ul);
+ }
+
+ memory[1] = 0;
+
+ {
+ int result = invoke<int>(*binary, memory, 1, ptr);
+
+ CHECK_EQ(result, 42);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], 10ul);
+ CHECK_EQ(memory[2], 0ul);
+ }
+
+ memory[1] = 0;
+
+ delete[] memory;
+}
+
+void testStoreAfterClobberDifferentWidth()
+{
+ Procedure proc;
+ GPRReg pinnedBaseGPR = GPRInfo::argumentGPR0;
+ proc.pinRegister(pinnedBaseGPR);
+
+ BasicBlock* root = proc.addBlock();
+
+ auto* pointer = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
+ auto* resultAddress = root->appendNew<WasmAddressValue>(proc, Origin(), pointer, pinnedBaseGPR);
+ root->appendNew<MemoryValue>(proc, Store, Origin(), root->appendNew<Const64Value>(proc, Origin(), -1), resultAddress, 0);
+ root->appendNew<MemoryValue>(proc, Store, Origin(), root->appendNew<Const32Value>(proc, Origin(), 20), resultAddress, 0);
+ root->appendNewControlValue(proc, Return, Origin(), root->appendNew<Const32Value>(proc, Origin(), 30));
+
+ auto binary = compileProc(proc);
+
+ uint64_t* memory = new uint64_t[10];
+ uint64_t ptr = 1*8;
+
+ for (int i = 0; i < 10; ++i)
+ memory[i] = 0;
+
+ {
+ int result = invoke<int>(*binary, memory, ptr);
+
+ CHECK_EQ(result, 30);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], (0xFFFFFFFF00000000ul | 20ul));
+ CHECK_EQ(memory[2], 0ul);
+ }
+
+ delete[] memory;
+}
+
+void testStoreAfterClobberDifferentWidthSuccessor()
+{
+ Procedure proc;
+ GPRReg pinnedBaseGPR = GPRInfo::argumentGPR0;
+ proc.pinRegister(pinnedBaseGPR);
+
+ BasicBlock* root = proc.addBlock();
+ BasicBlock* a = proc.addBlock();
+ BasicBlock* b = proc.addBlock();
+ BasicBlock* c = proc.addBlock();
+ BasicBlock* continuation = proc.addBlock();
+
+ auto* pointer = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
+ auto* path = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR2);
+ auto* resultAddress = root->appendNew<WasmAddressValue>(proc, Origin(), pointer, pinnedBaseGPR);
+ root->appendNew<MemoryValue>(proc, Store, Origin(), root->appendNew<Const64Value>(proc, Origin(), -1), resultAddress, 0);
+
+ SwitchValue* switchValue = root->appendNew<SwitchValue>(proc, Origin(), path);
+ switchValue->setFallThrough(FrequentedBlock(c));
+ switchValue->appendCase(SwitchCase(0, FrequentedBlock(a)));
+ switchValue->appendCase(SwitchCase(1, FrequentedBlock(b)));
+
+ a->appendNew<MemoryValue>(proc, Store, Origin(), a->appendNew<Const32Value>(proc, Origin(), 10), resultAddress, 0);
+ b->appendNew<MemoryValue>(proc, Store, Origin(), b->appendNew<Const32Value>(proc, Origin(), 20), resultAddress, 0);
+ c->appendNew<MemoryValue>(proc, Store, Origin(), c->appendNew<Const32Value>(proc, Origin(), 30), resultAddress, 0);
+
+ for (auto* i : { a, b, c }) {
+ i->appendNewControlValue(proc, Jump, Origin(), FrequentedBlock(continuation));
+ i->setSuccessors(FrequentedBlock(continuation));
+ }
+
+ continuation->appendNewControlValue(proc, Return, Origin(), continuation->appendNew<Const32Value>(proc, Origin(), 40));
+
+ auto binary = compileProc(proc);
+
+ uint64_t* memory = new uint64_t[10];
+ uint64_t ptr = 1*8;
+
+ for (int i = 0; i < 10; ++i)
+ memory[i] = 0;
+
+ {
+ int result = invoke<int>(*binary, memory, ptr, 0);
+
+ CHECK_EQ(result, 40);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], (0xFFFFFFFF00000000ul | 10ul));
+ CHECK_EQ(memory[2], 0ul);
+ }
+
+ memory[1] = 0;
+
+ {
+ int result = invoke<int>(*binary, memory, ptr, 1);
+
+ CHECK_EQ(result, 40);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], (0xFFFFFFFF00000000ul | 20ul));
+ CHECK_EQ(memory[2], 0ul);
+ }
+
+ memory[1] = 0;
+
+ {
+ int result = invoke<int>(*binary, memory, ptr, 2);
+
+ CHECK_EQ(result, 40);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], (0xFFFFFFFF00000000ul | 30ul));
+ CHECK_EQ(memory[2], 0ul);
+ }
+
+ delete[] memory;
+}
+
+void testStoreAfterClobberExitsSidewaysSuccessor()
+{
+ Procedure proc;
+ GPRReg pinnedBaseGPR = GPRInfo::argumentGPR0;
+ GPRReg pinnedSizeGPR = GPRInfo::argumentGPR1;
+ proc.pinRegister(pinnedBaseGPR);
+ proc.pinRegister(pinnedSizeGPR);
+
+ // Please don't make me save anything.
+ RegisterSet csrs;
+ csrs.merge(RegisterSet::calleeSaveRegisters());
+ csrs.exclude(RegisterSet::stackRegisters());
+ csrs.forEach(
+ [&] (Reg reg) {
+ CHECK(reg != pinnedBaseGPR);
+ CHECK(reg != pinnedSizeGPR);
+ proc.pinRegister(reg);
+ });
+
+ proc.setWasmBoundsCheckGenerator([=] (CCallHelpers& jit, GPRReg pinnedGPR) {
+ CHECK_EQ(pinnedGPR, pinnedSizeGPR);
+
+ jit.move(CCallHelpers::TrustedImm32(42), GPRInfo::returnValueGPR);
+ jit.emitFunctionEpilogue();
+ jit.ret();
+ });
+
+ BasicBlock* root = proc.addBlock();
+ BasicBlock* a = proc.addBlock();
+ BasicBlock* b = proc.addBlock();
+ BasicBlock* c = proc.addBlock();
+ BasicBlock* continuation = proc.addBlock();
+
+ auto* pointer = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR2);
+ auto* path = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR3);
+ auto* resultAddress = root->appendNew<WasmAddressValue>(proc, Origin(), pointer, pinnedBaseGPR);
+ root->appendNew<MemoryValue>(proc, Store, Origin(), root->appendNew<Const64Value>(proc, Origin(), -1), resultAddress, 0);
+
+ SwitchValue* switchValue = root->appendNew<SwitchValue>(proc, Origin(), path);
+ switchValue->setFallThrough(FrequentedBlock(c));
+ switchValue->appendCase(SwitchCase(0, FrequentedBlock(a)));
+ switchValue->appendCase(SwitchCase(1, FrequentedBlock(b)));
+
+ b->appendNew<WasmBoundsCheckValue>(proc, Origin(), pinnedSizeGPR, b->appendNew<Value>(proc, Trunc, Origin(), pointer), 0);
+
+ UpsilonValue* takeA = a->appendNew<UpsilonValue>(proc, Origin(), a->appendNew<Const64Value>(proc, Origin(), 10));
+ UpsilonValue* takeB = b->appendNew<UpsilonValue>(proc, Origin(), b->appendNew<Const64Value>(proc, Origin(), 20));
+ UpsilonValue* takeC = c->appendNew<UpsilonValue>(proc, Origin(), c->appendNew<Const64Value>(proc, Origin(), 30));
+
+ for (auto* i : { a, b, c }) {
+ i->appendNewControlValue(proc, Jump, Origin(), FrequentedBlock(continuation));
+ i->setSuccessors(FrequentedBlock(continuation));
+ }
+
+ auto* takenPhi = continuation->appendNew<Value>(proc, Phi, Int64, Origin());
+ continuation->appendNew<MemoryValue>(proc, Store, Origin(), takenPhi, resultAddress, 0);
+ continuation->appendNewControlValue(proc, Return, Origin(), continuation->appendNew<Const32Value>(proc, Origin(), 40));
+
+ takeA->setPhi(takenPhi);
+ takeB->setPhi(takenPhi);
+ takeC->setPhi(takenPhi);
+
+ auto binary = compileProc(proc);
+
+ uint64_t* memory = new uint64_t[10];
+ uint64_t ptr = 1*8;
+
+ for (int i = 0; i < 10; ++i)
+ memory[i] = 0;
+
+ {
+ int result = invoke<int>(*binary, memory, 16, ptr, 0);
+
+ CHECK_EQ(result, 40);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], 10ul);
+ CHECK_EQ(memory[2], 0ul);
+ }
+
+ memory[1] = 0;
+
+ {
+ int result = invoke<int>(*binary, memory, 16, ptr, 1);
+
+ CHECK_EQ(result, 40);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], 20ul);
+ CHECK_EQ(memory[2], 0ul);
+ }
+
+ memory[1] = 0;
+
+ {
+ int result = invoke<int>(*binary, memory, 16, ptr, 2);
+
+ CHECK_EQ(result, 40);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], 30ul);
+ CHECK_EQ(memory[2], 0ul);
+ }
+
+ memory[1] = 0;
+
+ {
+ int result = invoke<int>(*binary, memory, 1, ptr, 2);
+
+ CHECK_EQ(result, 40);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], 30ul);
+ CHECK_EQ(memory[2], 0ul);
+ }
+
+ memory[1] = 0;
+
+ {
+ int result = invoke<int>(*binary, memory, 1, ptr, 1);
+
+ CHECK_EQ(result, 42);
+ CHECK_EQ(memory[0], 0ul);
+ CHECK_EQ(memory[1], (0xFFFFFFFFFFFFFFFFul));
+ CHECK_EQ(memory[2], 0ul);
+ }
+
+ delete[] memory;
+}
+
void addCopyTests(const char* filter, Deque<RefPtr<SharedTask<void()>>>& tasks)
{
RUN(testFastForwardCopy32());
Modified: branches/safari-613-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (292989 => 292990)
--- branches/safari-613-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2022-04-19 00:49:38 UTC (rev 292989)
+++ branches/safari-613-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2022-04-19 00:49:42 UTC (rev 292990)
@@ -16277,6 +16277,7 @@
patchpoint->effects = Effects::none();
patchpoint->effects.exitsSideways = true;
patchpoint->effects.writesLocalState = true;
+ patchpoint->effects.reads = HeapRange::top();
patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
auto restore = [&] {
jit.popToRestore(GPRInfo::regT2);
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes