[webkit-changes] [292475] trunk/Source/JavaScriptCore

Wed, 06 Apr 2022 10:06:03 -0700

Title: [292475] trunk/Source/_javascript_Core
Revision
292475
Author
justin_mich...@apple.com
Date
2022-04-06 10:05:13 -0700 (Wed, 06 Apr 2022)

Log Message

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):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (292474 => 292475)


--- trunk/Source/_javascript_Core/ChangeLog	2022-04-06 16:36:06 UTC (rev 292474)
+++ trunk/Source/_javascript_Core/ChangeLog	2022-04-06 17:05:13 UTC (rev 292475)
@@ -1,3 +1,30 @@
+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-06  Xan Lopez  <x...@igalia.com>
 
         [JSC] Add DoNotHaveTagRegisters mode to unboxDouble

Modified: trunk/Source/_javascript_Core/b3/B3EliminateCommonSubexpressions.cpp (292474 => 292475)


--- trunk/Source/_javascript_Core/b3/B3EliminateCommonSubexpressions.cpp	2022-04-06 16:36:06 UTC (rev 292474)
+++ trunk/Source/_javascript_Core/b3/B3EliminateCommonSubexpressions.cpp	2022-04-06 17:05:13 UTC (rev 292475)
@@ -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: trunk/Source/_javascript_Core/b3/B3Validate.cpp (292474 => 292475)


--- trunk/Source/_javascript_Core/b3/B3Validate.cpp	2022-04-06 16:36:06 UTC (rev 292474)
+++ trunk/Source/_javascript_Core/b3/B3Validate.cpp	2022-04-06 17:05:13 UTC (rev 292475)
@@ -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: trunk/Source/_javascript_Core/b3/B3Value.cpp (292474 => 292475)


--- trunk/Source/_javascript_Core/b3/B3Value.cpp	2022-04-06 16:36:06 UTC (rev 292474)
+++ trunk/Source/_javascript_Core/b3/B3Value.cpp	2022-04-06 17:05:13 UTC (rev 292475)
@@ -669,6 +669,7 @@
             break;
         }
         result.exitsSideways = true;
+        result.reads = HeapRange::top();
         break;
     case Upsilon:
     case Set:

Modified: trunk/Source/_javascript_Core/b3/testb3.h (292474 => 292475)


--- trunk/Source/_javascript_Core/b3/testb3.h	2022-04-06 16:36:06 UTC (rev 292474)
+++ trunk/Source/_javascript_Core/b3/testb3.h	2022-04-06 17:05:13 UTC (rev 292475)
@@ -1176,4 +1176,10 @@
 void testFloatMaxMin();
 void testDoubleMaxMin();
 
+void testWasmAddressDoesNotCSE();
+void testStoreAfterClobberExitsSideways();
+void testStoreAfterClobberDifferentWidth();
+void testStoreAfterClobberDifferentWidthSuccessor();
+void testStoreAfterClobberExitsSidewaysSuccessor();
+
 #endif // ENABLE(B3_JIT)

Modified: trunk/Source/_javascript_Core/b3/testb3_1.cpp (292474 => 292475)


--- trunk/Source/_javascript_Core/b3/testb3_1.cpp	2022-04-06 16:36:06 UTC (rev 292474)
+++ trunk/Source/_javascript_Core/b3/testb3_1.cpp	2022-04-06 17:05:13 UTC (rev 292475)
@@ -541,6 +541,12 @@
     RUN(testSpillGP());
     RUN(testSpillFP());
 
+    RUN(testWasmAddressDoesNotCSE());
+    RUN(testStoreAfterClobberDifferentWidth());
+    RUN(testStoreAfterClobberExitsSideways());
+    RUN(testStoreAfterClobberDifferentWidthSuccessor());
+    RUN(testStoreAfterClobberExitsSidewaysSuccessor());
+
     RUN(testInt32ToDoublePartialRegisterStall());
     RUN(testInt32ToDoublePartialRegisterWithoutStall());

Modified: trunk/Source/_javascript_Core/b3/testb3_7.cpp (292474 => 292475)


--- trunk/Source/_javascript_Core/b3/testb3_7.cpp	2022-04-06 16:36:06 UTC (rev 292474)
+++ trunk/Source/_javascript_Core/b3/testb3_7.cpp	2022-04-06 17:05:13 UTC (rev 292475)
@@ -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: trunk/Source/_javascript_Core/b3/testb3_8.cpp (292474 => 292475)


--- trunk/Source/_javascript_Core/b3/testb3_8.cpp	2022-04-06 16:36:06 UTC (rev 292474)
+++ trunk/Source/_javascript_Core/b3/testb3_8.cpp	2022-04-06 17:05:13 UTC (rev 292475)
@@ -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: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (292474 => 292475)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2022-04-06 16:36:06 UTC (rev 292474)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2022-04-06 17:05:13 UTC (rev 292475)
@@ -16309,6 +16309,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

Reply via email to