Cherry-pick 272448.698@safari-7618-branch (e6aa0aa). https://bugs.web…

…kit.org/show_bug.cgi?id=265426

    [JSC] B3::EliminateCommonSubexpression shouldn't remove reads info after processing each block
    https://bugs.webkit.org/show_bug.cgi?id=265426
    rdar://118832222

    Reviewed by Yusuke Suzuki.

    Eliminate common subexpressions in B3 is used to remove redundant B3 nodes.
    Current algorithm removes block reads info after processing each block. This is wrong
    since some B3 nodes may be deleted erroneously due to the missing reads info from
    the processed blocks. To fix this issue, we should update block reads info after
    processing each node.

    * JSTests/stress/ecs-store-with-loop.js: Added.
    (foo):
    (main):
    * Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp:
    * Source/JavaScriptCore/b3/testb3.h:
    * Source/JavaScriptCore/b3/testb3_3.cpp:
    (testCSEStoreWithLoop):
    (addShrTests):

    Canonical link: https://commits.webkit.org/272448.698@safari-7618-branch

Canonical link: https://commits.webkit.org/274313.229@webkitglib/2.44

JSTests/stress/ecs-store-with-loop.js

@@ -0,0 +1,24 @@
+//@ runDefault("--jitPolicyScale=0")
+let var1 = 0x80000000;
+
+function foo() {
+    for (let i = 0; i < 7; i++) {
+        var1 = var1 << i;
+        var1 = var1 - 1;
+    }
+    var1 >>= 8;
+    return var1;
+}
+noInline(foo);
+
+function main() {
+    let ret = undefined;
+    foo();
+    let expected = foo();
+    for (let i = 0; i < 1e3; i++) {
+        ret = foo();
+        if (expected != ret)
+            throw new Error();
+    }
+}
+main();

Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp

@@ -278,6 +278,12 @@ class CSE {
 
         if (memory)
             processMemoryAfterClobber(memory);
+
+        // The reads info should be updated even the block is processed
+        // since the dominated store nodes may dependent on the data
+        // read from the processed block. Note that there is no need to
+        // update reads info if the node is deleted.
+        m_data.reads.add(m_value->effects().reads);
     }
 
     // Return true if we got rid of the operation. If you changed IR in this function, you have to

Source/JavaScriptCore/b3/testb3.h

@@ -1202,6 +1202,8 @@ void addTupleTests(const TestConfig*, Deque<RefPtr<SharedTask<void()>>>&);
 
 bool shouldRun(const TestConfig*, const char* testName);
 
+void testCSEStoreWithLoop();
+
 void testLoadPreIndex32();
 void testLoadPreIndex64();
 void testLoadPostIndex32();

Source/JavaScriptCore/b3/testb3_3.cpp

@@ -28,6 +28,59 @@
 
 #if ENABLE(B3_JIT) && !CPU(ARM)
 
+void testCSEStoreWithLoop()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    BasicBlock* loop = proc.addBlock();
+    BasicBlock* body = proc.addBlock();
+    BasicBlock* done = proc.addBlock();
+
+    // --------------------------- Root ---------------------------
+    Value* constZero = root->appendIntConstant(proc, Origin(), Int64, 0);
+    Value* constOne = root->appendIntConstant(proc, Origin(), Int64, 1);
+    Value* constTen = root->appendIntConstant(proc, Origin(), Int64, 10);
+    Value* address = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+    Value* count = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
+    UpsilonValue* originalCounter = root->appendNew<UpsilonValue>(proc, Origin(), constZero);
+    root->appendNewControlValue(proc, Jump, Origin(), FrequentedBlock(loop));
+
+    // --------------------------- Loop ---------------------------
+    Value* loopCounter = loop->appendNew<Value>(proc, Phi, Int64, Origin());
+    Value* incCounter = loop->appendNew<Value>(proc, Add, Origin(), loopCounter, constOne);
+    UpsilonValue* incCounterUpsilon = loop->appendNew<UpsilonValue>(proc, Origin(), incCounter);
+    originalCounter->setPhi(loopCounter);
+    incCounterUpsilon->setPhi(loopCounter);
+    Value* valueFromAddress = loop->appendNew<MemoryValue>(proc, Load, Int64, Origin(), address);
+    loop->appendNewControlValue(proc, Branch, Origin(),
+        loop->appendNew<Value>(proc, Below, Origin(), incCounter, constTen),
+        FrequentedBlock(body),
+        FrequentedBlock(loop));
+
+    // --------------------------- Body ---------------------------
+    body->appendNew<MemoryValue>(proc, Store, Origin(), count, address);
+    CheckValue* checkAdd = body->appendNew<CheckValue>(proc, CheckAdd, Origin(), count, valueFromAddress);
+    checkAdd->setGenerator(
+        [&](CCallHelpers& jit, const StackmapGenerationParams&) {
+            AllowMacroScratchRegisterUsage allowScratch(jit);
+            jit.abortWithReason(B3Oops);
+        });
+    body->appendNew<MemoryValue>(proc, Store, Origin(), checkAdd, address);
+    body->appendNewControlValue(proc, Branch, Origin(),
+        body->appendNew<Value>(proc, Below, Origin(), incCounter, count),
+        FrequentedBlock(loop),
+        FrequentedBlock(done));
+
+    // --------------------------- Done ---------------------------
+    done->appendNew<MemoryValue>(proc, Store, Origin(), checkAdd, address);
+    done->appendNewControlValue(proc, Return, Origin(), address);
+
+
+    auto code = compileProc(proc);
+    int64_t num = 1;
+    invoke<int64_t>(*code, bitwise_cast<intptr_t>(&num), 2);
+    CHECK_EQ(num, 5);
+}
 
 void testLoadPreIndex32()
 {
@@ -4099,6 +4152,7 @@ void addShrTests(const TestConfig* config, Deque<RefPtr<SharedTask<void()>>>& ta
     RUN(testZShrArgImm32(0xffffffff, 0));
     RUN(testZShrArgImm32(0xffffffff, 1));
     RUN(testZShrArgImm32(0xffffffff, 63));
+    RUN(testCSEStoreWithLoop());
 
     if (Options::useB3CanonicalizePrePostIncrements()) {
         RUN(testLoadPreIndex32());