fix[venom]: fix sccp TOP evaluation order (#4748)

This commit updates SCCP to not assert during arithmetic evaluation 
if any operand’s lattice value was not a literal, and instead 
`SCCP._eval` returns TOP when any operand is TOP (defers 
constant folding) and keep existing semantics for BOTTOM and 
literals. Adds a test that triggers the assertion without the fix. 
Reschedule instructions in generic case when setting to BOTTOM.

Author: harkal

tests/unit/compiler/venom/test_sccp.py

@@ -2,7 +2,9 @@
 
 from tests.venom_utils import PrePostChecker
 from vyper.exceptions import StaticAssertionException
+from vyper.venom.analysis.analysis import IRAnalysesCache
 from vyper.venom.basicblock import IRVariable
+from vyper.venom.parser import parse_venom
 from vyper.venom.passes import SCCP
 from vyper.venom.passes.sccp.sccp import LatticeEnum
 
@@ -278,6 +280,75 @@ def test_cont_phi_const_case():
     assert sccp.lattice[IRVariable("%5:2")] == LatticeEnum.TOP
 
 
+def test_sccp_phi_operand_top_no_branch():
+    """
+    control jumps directly to a join block where a phi depends on predecessors
+    that haven't been executed yet. The phi is TOP at first, and hhe arithmetic
+    using it must defer evaluation.
+    """
+    # NOTE: `main` goes straight to `@join`, yet the phi still lists `@then`
+    # and `@else` as inputs. This intentionally mimics malformed IR seen in
+    # programs where the CFG includes those predecessors even though
+    # execution never reaches them (and will be prunned by a later pass).
+    # So here we show that can SCCP gracefully treat the phi inputs
+    # as TOP until (and unless) those blocks are actually visited. Decoupling
+    # essentially the CGF from the SCCP.
+    pre = """
+    main:
+        jmp @join
+    then:
+        %a_then = 2
+        jmp @join
+    else:
+        %a_else = 3
+        jmp @join
+    join:
+        %phi = phi @then, %a_then, @else, %a_else
+        %out = sub 14, %phi
+        sink %out
+    """
+
+    _check_pre_post(pre, pre, hevm=False)
+
+
+def test_sccp_jnz_top_phi_text_ir():
+    """
+    Same as above but using the value to control a jnz.
+    This used to assert in SCCP when the jnz condition was TOP.
+    """
+    # NOTE: `main` goes straight to `@join`, yet the phi still lists `@then`
+    # and `@else` as inputs. This intentionally mimics malformed IR seen in
+    # programs where the CFG includes those predecessors even though
+    # execution never reaches them (and will be prunned by a later pass).
+    # So here we show that can SCCP gracefully treat the phi inputs
+    # as TOP until (and unless) those blocks are actually visited. Decoupling
+    # essentially the CGF from the SCCP.
+    src = """
+    function main {
+    main:
+        jmp @join
+    then:
+        %a_then = 2
+        jmp @join
+    else:
+        %a_else = 3
+        jmp @join
+    join:
+        %phi = phi @then, %a_then, @else, %a_else
+        jnz %phi, @true, @false
+    true:
+        sink 1
+    false:
+        sink 2
+    }
+    """
+
+    ctx = parse_venom(src)
+    fn = ctx.get_function(next(iter(ctx.functions.keys())))
+    ac = IRAnalysesCache(fn)
+    SCCP(ac, fn).run_pass()
+
+
 def test_phi_reduction_without_basic_block_removal():
     """
     Test of phi reduction `if` end not `if-else`
@@ -307,3 +378,30 @@ def test_phi_reduction_without_basic_block_removal():
     """
 
     _check_pre_post(pre, post)
+
+
+inst = ["mload", "sload", "dload", "iload", "calldataload", "param"]
+
+
+@pytest.mark.parametrize("inst", inst)
+def test_mload_schedules_uses(inst):
+    pre = f"""
+    main:
+        %cond = param
+        jnz %cond, @B, @A
+    A:
+        %m = {inst} 0
+        jmp @join
+    B:
+        %x = assign %m
+        jmp @join
+    join:
+        sink %x
+    """
+
+    passes = _check_pre_post(pre, pre, hevm=False)
+    sccp = passes[0]
+    assert isinstance(sccp, SCCP)
+
+    assert sccp.lattice[IRVariable("%m")] == LatticeEnum.BOTTOM
+    assert sccp.lattice[IRVariable("%x")] == LatticeEnum.BOTTOM

vyper/venom/passes/sccp/sccp.py

@@ -195,8 +195,7 @@ def _visit_expr(self, inst: IRInstruction):
         elif opcode == "jnz":
             lat = self._eval_from_lattice(inst.operands[0])
 
-            assert lat != LatticeEnum.TOP, f"Got undefined var at jmp at {inst.parent}"
-            if lat == LatticeEnum.BOTTOM:
+            if lat in (LatticeEnum.BOTTOM, LatticeEnum.TOP):
                 for out_bb in self.cfg.cfg_out(inst.parent):
                     self.work_list.append(FlowWorkItem(inst.parent, out_bb))
             else:
@@ -210,24 +209,18 @@ def _visit_expr(self, inst: IRInstruction):
                 self.work_list.append(FlowWorkItem(inst.parent, target))
         elif opcode == "djmp":
             lat = self._eval_from_lattice(inst.operands[0])
-            assert lat != LatticeEnum.TOP, f"Got undefined var at jmp at {inst.parent}"
-            if lat == LatticeEnum.BOTTOM:
+            if lat in (LatticeEnum.BOTTOM, LatticeEnum.TOP):
                 for op in inst.operands[1:]:
                     target = self.fn.get_basic_block(op.name)
                     self.work_list.append(FlowWorkItem(inst.parent, target))
             elif isinstance(lat, IRLiteral):
                 raise CompilerPanic("Unimplemented djmp with literal")
-
-        elif opcode in ["param", "calldataload"]:
-            self.lattice[inst.output] = LatticeEnum.BOTTOM  # type: ignore
-            self._add_ssa_work_items(inst)
-        elif opcode == "mload":
-            self.lattice[inst.output] = LatticeEnum.BOTTOM  # type: ignore
         elif opcode in ARITHMETIC_OPS:
             self._eval(inst)
         else:
             if inst.output is not None:
                 self._set_lattice(inst.output, LatticeEnum.BOTTOM)
+                self._add_ssa_work_items(inst)
 
     def _eval(self, inst) -> LatticeItem:
         """
@@ -257,12 +250,15 @@ def finalize(ret):
                 assert isinstance(op, IRLiteral)  # clarity
                 eval_result = op
 
-            # The value from the lattice should have evaluated to BOTTOM
-            # or a literal by now.
+            # The value from the lattice should have evaluated to TOP,
+            # BOTTOM, or a literal by now.
+
             # If any operand is BOTTOM, the whole operation is BOTTOM
-            # and we can stop the evaluation early
             if eval_result is LatticeEnum.BOTTOM:
                 return finalize(LatticeEnum.BOTTOM)
+            # If any operand is TOP the operation is TOP
+            if eval_result is LatticeEnum.TOP:
+                return finalize(LatticeEnum.TOP)
 
             assert isinstance(eval_result, IRLiteral), (inst.parent.label, op, inst)
             ops.append(eval_result)