fix[venom]: fix incorrect write reordering in DFTPass

tests/functional/codegen/features/iteration/test_bubble_sort.py

@@ -0,0 +1,64 @@
+"""
+Test vyper implementation of bubble sort. Good functional test as it
+stresses the code generator and optimizer a little.
+"""
+
+
+def test_bubble_sort(get_contract):
+    code = """
+MAX_DATA_SIZE: constant(uint256) = 100
+
+data: DynArray[uint256, MAX_DATA_SIZE]
+
+@internal
+@view
+def _validate_index(idx: uint256):
+    assert idx < len(self.data), "Index out of bounds"
+
+@internal
+def _swap(i: uint256, j: uint256):
+    self._validate_index(i)
+    self._validate_index(j)
+    temp: uint256 = self.data[i]
+    self.data[i] = self.data[j]
+    self.data[j] = temp
+
+@internal
+def _bubble_sort():
+    n: uint256 = len(self.data)
+    for i: uint256 in range(n, bound=MAX_DATA_SIZE):
+        for j: uint256 in range(n - i - 1, bound=MAX_DATA_SIZE):
+            if self.data[j] > self.data[j + 1]:
+                self._swap(j, j + 1)
+
+@external
+def add(val: uint256):
+    self.data.append(val)
+
+@external
+def sort_data():
+    self._bubble_sort()
+
+@external
+@view
+def get(idx: uint256) -> uint256:
+    self._validate_index(idx)
+    return self.data[idx]
+"""
+
+    c = get_contract(code)
+
+    # add unsorted data
+    c.add(5)
+    c.add(2)
+    c.add(8)
+    c.add(1)
+
+    # sort
+    c.sort_data()
+
+    # check sorted
+    results = [c.get(i) for i in range(4)]
+    print(f"After sorting: {results}")
+
+    assert results == [1, 2, 5, 8]

tests/unit/compiler/venom/test_dft_write_after_write.py

@@ -0,0 +1,50 @@
+import pytest
+
+from vyper.venom.analysis import IRAnalysesCache
+from vyper.venom.basicblock import IRLabel
+from vyper.venom.parser import parse_venom
+from vyper.venom.passes import DFTPass
+
+
+@pytest.mark.parametrize(
+    "store_op,terminator",
+    [
+        ("sstore", "stop"),  # storage
+        ("mstore", "return 0, 32"),  # memory
+        ("tstore", "stop"),  # transient storage
+    ],
+)
+def test_write_after_write_dependency(store_op, terminator):
+    """
+    Test that DFT pass does not reorder writes
+    """
+    source = f"""
+    function test {{
+        test:
+            %x = param
+            %y = param
+            {store_op} 0, %x       ; first write
+            {store_op} 1, %y       ; second write to different slot
+            {store_op} 0, %y       ; third write overwrites first
+            {terminator}
+    }}
+    """
+
+    ctx = parse_venom(source)
+    fn = ctx.get_function(IRLabel("test"))
+
+    ac = IRAnalysesCache(fn)
+    DFTPass(ac, fn).run_pass()
+
+    bb = fn.get_basic_block("test")
+    instructions = bb.instructions
+
+    # find indices of store instructions to slot/location 0
+    store0_indices = []
+
+    for i, inst in enumerate(instructions):
+        if inst.opcode == store_op and inst.operands[1].value == 0:
+            store0_indices.append(i)
+
+    assert len(store0_indices) == 2
+    assert store0_indices[0] < store0_indices[1], f"Write order must be preserved for {store_op}"

vyper/venom/__init__.py

@@ -79,6 +79,7 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel, ac: IRAnalysesCache
     SimplifyCFGPass(ac, fn).run_pass()
     AssignElimination(ac, fn).run_pass()
     AlgebraicOptimizationPass(ac, fn).run_pass()
+
     LoadElimination(ac, fn).run_pass()
 
     SCCP(ac, fn).run_pass()
@@ -104,6 +105,7 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel, ac: IRAnalysesCache
     PhiEliminationPass(ac, fn).run_pass()
     AssignElimination(ac, fn).run_pass()
     CSE(ac, fn).run_pass()
+
     AssignElimination(ac, fn).run_pass()
     RemoveUnusedVariablesPass(ac, fn).run_pass()
     SingleUseExpansion(ac, fn).run_pass()

vyper/venom/passes/dft.py

@@ -108,6 +108,9 @@ def _calculate_dependency_graphs(self, bb: IRBasicBlock) -> None:
             for write_effect in write_effects:
                 if write_effect in last_read_effects:
                     self.eda[inst].add(last_read_effects[write_effect])
+                # prevent reordering write-after-write for the same effect
+                if write_effect in last_write_effects:
+                    self.eda[inst].add(last_write_effects[write_effect])
                 last_write_effects[write_effect] = inst
 
             for read_effect in read_effects: