feat[venom]: improve phi elimination pass (#4635)

this commit improves the phi elimination pass (introduced in
6b670e7) when there are multiple phis in the chain. if the
innermost phi has multiple origins, it would block elimination of outer
phis which use that phi. for example,

```llvm
%a = 1
%b = 2
%c = phi %a, %b  ; has two origins
%d = %c
%e = %d
%f = phi %d, %e
```

prior to this commit, `%f = phi %d, %e` would not reduce, because its
origins were detected as `%a = 1` *and* `%b = 2`. this commit modifies
the graph traversal so that further traversal of `%c`'s origins is
blocked from the perspective of `%f`, allowing `%f` to be reduced to
`%c`.

---------

Co-authored-by: Charles Cooper <[email protected]>

Author: HodanPlodky

tests/unit/compiler/venom/test_phi_elimination.py

@@ -212,6 +212,7 @@ def test_phi_elim_cannot_remove():
     main:
         %p = param
         %rand = param
+        jmp @cond
     cond:
         %1 = phi @main, %p, @body, %3
         %cond = iszero %1
@@ -264,3 +265,57 @@ def test_phi_elim_direct_loop():
 
     _check_pre_post(pre1, post)
     _check_pre_post(pre2, post)
+
+
+def test_phi_elim_two_phi_merges():
+    pre = """
+    main:
+        %cond = param
+        %cond2 = param
+        jnz %cond, @1_then, @2_then
+    1_then:
+        %1 = 100
+        jmp @3_join
+    2_then:
+        %2 = 101
+        jmp @3_join
+    3_join:
+        %3 = phi @1_then, %1, @2_then, %2
+        jnz %cond2, @4_then, @5_then
+    4_then:
+        %4 = %3
+        jmp @6_join
+    5_then:
+        %5 = %3
+        jmp @6_join
+    6_join:
+        %6 = phi @4_then, %4, @5_then, %5  ; should be reduced to %3!
+        sink %6
+    """
+
+    post = """
+    main:
+        %cond = param
+        %cond2 = param
+        jnz %cond, @1_then, @2_then
+    1_then:
+        %1 = 100
+        jmp @3_join
+    2_then:
+        %2 = 101
+        jmp @3_join
+    3_join:
+        %3 = phi @1_then, %1, @2_then, %2
+        jnz %cond2, @4_then, @5_then
+    4_then:
+        %4 = %3
+        jmp @6_join
+    5_then:
+        %5 = %3
+        jmp @6_join
+    6_join:
+        %6 = %3
+        sink %6
+    """
+
+    _check_pre_post(pre, post, hevm=True)

vyper/venom/__init__.py

@@ -60,11 +60,13 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel, ac: IRAnalysesCache
     SimplifyCFGPass(ac, fn).run_pass()
 
     MakeSSA(ac, fn).run_pass()
+    PhiEliminationPass(ac, fn).run_pass()
     # run algebraic opts before mem2var to reduce some pointer arithmetic
     AlgebraicOptimizationPass(ac, fn).run_pass()
     AssignElimination(ac, fn).run_pass()
     Mem2Var(ac, fn).run_pass()
     MakeSSA(ac, fn).run_pass()
+    PhiEliminationPass(ac, fn).run_pass()
     SCCP(ac, fn).run_pass()
 
     SimplifyCFGPass(ac, fn).run_pass()

vyper/venom/analysis/__init__.py

@@ -6,6 +6,5 @@
 from .liveness import LivenessAnalysis
 from .mem_alias import MemoryAliasAnalysis
 from .mem_ssa import MemSSA
-from .phi_reach import PhiReachingAnalysis
 from .reachable import ReachableAnalysis
 from .var_definition import VarDefinition

vyper/venom/analysis/phi_reach.py

@@ -1,30 +1,97 @@
-from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, PhiReachingAnalysis
-from vyper.venom.basicblock import IRInstruction
+from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis
+from vyper.venom.basicblock import IRInstruction, IRVariable
 from vyper.venom.passes.base_pass import InstUpdater, IRPass
 
 
 class PhiEliminationPass(IRPass):
-    phi_reach: PhiReachingAnalysis
+    phi_to_origins: dict[IRInstruction, set[IRInstruction]]
 
     def run_pass(self):
         self.dfg = self.analyses_cache.request_analysis(DFGAnalysis)
         self.updater = InstUpdater(self.dfg)
-        self.phi_reach = self.analyses_cache.request_analysis(PhiReachingAnalysis)
+        self._calculate_phi_origins()
 
         for _, inst in self.dfg.outputs.copy().items():
             if inst.opcode != "phi":
                 continue
             self._process_phi(inst)
 
+        # sort phis to top of basic block
         for bb in self.function.get_basic_blocks():
             bb.ensure_well_formed()
 
         self.analyses_cache.invalidate_analysis(LivenessAnalysis)
 
     def _process_phi(self, inst: IRInstruction):
-        srcs = self.phi_reach.phi_to_origins[inst]
+        srcs = self.phi_to_origins[inst]
 
         if len(srcs) == 1:
-            src = next(iter(srcs))
+            src = srcs.pop()
+            if src == inst:
+                return
             assert src.output is not None
             self.updater.store(inst, src.output)
+
+    def _calculate_phi_origins(self):
+        self.dfg = self.analyses_cache.request_analysis(DFGAnalysis)
+        self.phi_to_origins = dict()
+
+        for bb in self.function.get_basic_blocks():
+            for inst in bb.instructions:
+                if inst.opcode != "phi":
+                    break
+                self._get_phi_origins(inst)
+
+    def _get_phi_origins(self, inst: IRInstruction):
+        assert inst.opcode == "phi"  # sanity
+        visited: set[IRInstruction] = set()
+        self.phi_to_origins[inst] = self._get_phi_origins_r(inst, visited)
+
+    # traverse chains of phis and stores to get the "root" instructions
+    # for phis.
+    def _get_phi_origins_r(
+        self, inst: IRInstruction, visited: set[IRInstruction]
+    ) -> set[IRInstruction]:
+        if inst.opcode == "phi":
+            if inst in self.phi_to_origins:
+                return self.phi_to_origins[inst]
+
+            if inst in visited:
+                # we have hit a dfg cycle. break the recursion.
+                # if it is only visited we have found a self
+                # reference, and we won't find anything more by
+                # continuing the recursion.
+                return set()
+
+            visited.add(inst)
+
+            res: set[IRInstruction] = set()
+
+            for _, var in inst.phi_operands:
+                next_inst = self.dfg.get_producing_instruction(var)
+                assert next_inst is not None, (inst, var)
+                res |= self._get_phi_origins_r(next_inst, visited)
+
+            if len(res) > 1:
+                # if this phi has more than one origin, then for future
+                # phis, it is better to treat this as a barrier in the
+                # graph traversal. for example (without basic blocks)
+                #   %a = 1
+                #   %b = 2
+                #   %c = phi %a, %b  ; has two origins
+                #   %d = %c
+                #   %e = %d
+                #   %f = phi %d, %e
+                # in this case, %f should reduce to %c.
+                return set([inst])
+            return res
+
+        if inst.opcode == "store" and isinstance(inst.operands[0], IRVariable):
+            # traverse store chain
+            var = inst.operands[0]
+            next_inst = self.dfg.get_producing_instruction(var)
+            assert next_inst is not None
+            return self._get_phi_origins_r(next_inst, visited)
+
+        # root of the phi/store chain
+        return set([inst])