[JSC] REGRESSION(269099@main) Fix the regression caused by HeapLocati…

…on with an extra state

https://bugs.webkit.org/show_bug.cgi?id=262979
rdar://116764992

Reviewed by Yusuke Suzuki.

Previously, we landed a patch (269099@main) to introduce a new HeapLocation
constructor with an extra state, which might disable certain optimizations in
DFG and FTL and cause regressions. This patch fixes the issue by enabling
the new HeapLocation construction only after LICM phase.

* Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:
* Source/JavaScriptCore/dfg/DFGCSEPhase.cpp:
(JSC::DFG::performGlobalCSE):
(JSC::DFG::performGlobalCSESlow):
* Source/JavaScriptCore/dfg/DFGCSEPhase.h:
* Source/JavaScriptCore/dfg/DFGClobberSet.cpp:
(JSC::DFG::addReads):
(JSC::DFG::addWrites):
(JSC::DFG::addReadsAndWrites):
(JSC::DFG::readsOverlap):
(JSC::DFG::writesOverlap):
* Source/JavaScriptCore/dfg/DFGClobberize.cpp:
(JSC::DFG::doesWrites):
(JSC::DFG::accessesOverlap):
(JSC::DFG::writesOverlap):
(JSC::DFG::clobbersHeap):
* Source/JavaScriptCore/dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* Source/JavaScriptCore/dfg/DFGClobbersExitState.cpp:
(JSC::DFG::clobbersExitState):
* Source/JavaScriptCore/dfg/DFGPlan.cpp:
(JSC::DFG::Plan::compileInThreadImpl):
* Source/JavaScriptCore/dfg/DFGPreciseLocalClobberize.h:
(JSC::DFG::preciseLocalClobberize):
* Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileCurrentBlock):
* Source/JavaScriptCore/dfg/DFGStoreBarrierInsertionPhase.cpp:
* Source/JavaScriptCore/dfg/DFGValidate.cpp:
* Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:
* Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileBlock):
(JSC::FTL::DFG::LowerDFGToB3::compileNode):

Canonical link: https://commits.webkit.org/269295@main

JSTests/stress/getbyoffset-cse-consistency-2.js

@@ -0,0 +1,45 @@
+
+let a = {"x": 3};
+a.y1 = 3.3;
+
+let b = {};
+let res = 42;
+b.__defineGetter__("y2", function () {
+    res = 43;
+    return {};
+});
+b.x = 1.1;
+
+let c = {};
+c.slot_1 = 1.1;
+
+function func(flag, arg0, arg1) {
+    let tmp = b;
+    if (flag) {
+        tmp = a;
+    }
+    if (arg1 > 100) {
+        return;
+    }
+    for (let i = 0; i < 2; i++) {
+        if (flag) {
+            var x = tmp.x;
+        } else {
+            var x = tmp.x;
+        }
+        arg0.y = 3.3;
+    }
+    "" + x;
+    c.slot_1 = x;
+}
+
+for (let i = 0; i < 0x100000; i++) {
+    func(i % 2, {}, i);
+}
+
+b.x
+func(0, {}, 1);
+c.slot_1
+
+if (res == 43)
+    throw "BAD!";

Source/JavaScriptCore/dfg/DFGClobberize.h

@@ -1476,7 +1476,17 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
         unsigned identifierNumber = node->storageAccessData().identifierNumber;
         AbstractHeap heap(NamedProperties, identifierNumber);
         read(heap);
-        def(HeapLocation(NamedPropertyLoc, heap, node->child2(), &node->storageAccessData()), LazyNode(node));
+
+        // Since LICM might break the uniqueness assumption of HeapLocation for
+        // *byOffset nodes. Then, the HeapLocation constructor with an extra state
+        // is introduced and applied in this phase in order to resolve the potential
+        // HeapLocation collisions for *byteOffset nodes after LICM phase. Note
+        // that the constructor with an extra state should be used only after LICM
+        // since it might affect performance.
+        if (graph.m_planStage >= PlanStage::LICMAndLater)
+            def(HeapLocation(NamedPropertyLoc, heap, node->child2(), &node->storageAccessData()), LazyNode(node));
+        else
+            def(HeapLocation(NamedPropertyLoc, heap, node->child2()), LazyNode(node));
         return;
     }
 
@@ -1485,7 +1495,10 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
         read(JSObject_butterfly);
         AbstractHeap heap(NamedProperties, node->multiGetByOffsetData().identifierNumber);
         read(heap);
-        def(HeapLocation(NamedPropertyLoc, heap, node->child1(), &node->multiGetByOffsetData()), LazyNode(node));
+        if (graph.m_planStage >= PlanStage::LICMAndLater)
+            def(HeapLocation(NamedPropertyLoc, heap, node->child1(), &node->multiGetByOffsetData()), LazyNode(node));
+        else
+            def(HeapLocation(NamedPropertyLoc, heap, node->child1()), LazyNode(node));
         return;
     }
 
@@ -1498,7 +1511,10 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
             write(JSCell_structureID);
         if (node->multiPutByOffsetData().reallocatesStorage())
             write(JSObject_butterfly);
-        def(HeapLocation(NamedPropertyLoc, heap, node->child1(), &node->multiPutByOffsetData()), LazyNode(node->child2().node()));
+        if (graph.m_planStage >= PlanStage::LICMAndLater)
+            def(HeapLocation(NamedPropertyLoc, heap, node->child1(), &node->multiPutByOffsetData()), LazyNode(node->child2().node()));
+        else
+            def(HeapLocation(NamedPropertyLoc, heap, node->child1()), LazyNode(node->child2().node()));
         return;
     }
 
@@ -1520,7 +1536,10 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
         unsigned identifierNumber = node->storageAccessData().identifierNumber;
         AbstractHeap heap(NamedProperties, identifierNumber);
         write(heap);
-        def(HeapLocation(NamedPropertyLoc, heap, node->child2(), &node->storageAccessData()), LazyNode(node->child3().node()));
+        if (graph.m_planStage >= PlanStage::LICMAndLater)
+            def(HeapLocation(NamedPropertyLoc, heap, node->child2(), &node->storageAccessData()), LazyNode(node->child3().node()));
+        else
+            def(HeapLocation(NamedPropertyLoc, heap, node->child2()), LazyNode(node->child3().node()));
         return;
     }
 

Source/JavaScriptCore/dfg/DFGCommon.h

@@ -252,7 +252,8 @@ inline KillStatus killStatusForDoesKill(bool doesKill)
 
 enum class PlanStage {
     Initial,
-    AfterFixup
+    AfterFixup,
+    LICMAndLater
 };
 
 // If possible, this will acquire a lock to make sure that if multiple threads

Source/JavaScriptCore/dfg/DFGLICMPhase.cpp

@@ -63,6 +63,9 @@ class LICMPhase : public Phase {
 
     bool run()
     {
+        ASSERT(m_graph.m_planStage <= PlanStage::LICMAndLater);
+        m_graph.m_planStage = PlanStage::LICMAndLater;
+
         DFG_ASSERT(m_graph, nullptr, m_graph.m_form == SSA);
 
         m_graph.ensureSSADominators();