Title: [292990] branches/safari-613-branch/Source/_javascript_Core
Revision
292990
Author
alanc...@apple.com
Date
2022-04-18 17:49:42 -0700 (Mon, 18 Apr 2022)

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

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

Reply via email to