diff --git a/tests/unit/compiler/venom/test_algebraic_optimizer.py b/tests/unit/compiler/venom/test_algebraic_optimizer.py index 39008649ea..00ccb0684a 100644 --- a/tests/unit/compiler/venom/test_algebraic_optimizer.py +++ b/tests/unit/compiler/venom/test_algebraic_optimizer.py @@ -1,5 +1,6 @@ import pytest +import vyper from vyper.venom.analysis import IRAnalysesCache from vyper.venom.basicblock import IRBasicBlock, IRLabel from vyper.venom.context import IRContext @@ -176,3 +177,29 @@ def test_offsets(): offset_count += 1 assert offset_count == 3 + + +# Test the case of https://github.com/vyperlang/vyper/issues/4288 +def test_ssa_after_algebraic_optimization(): + code = """ +@internal +def _do_math(x: uint256) -> uint256: + value: uint256 = x + result: uint256 = 0 + + if (x >> 128 != 0): + x >>= 128 + if (x >> 64 != 0): + x >>= 64 + + if 1 < value: + result = 1 + + return result + +@external +def run() -> uint256: + return self._do_math(10) + """ + + vyper.compile_code(code, output_formats=["bytecode"]) diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 310147baa7..bf3115b4dd 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -55,6 +55,13 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: StoreElimination(ac, fn).run_pass() SimplifyCFGPass(ac, fn).run_pass() AlgebraicOptimizationPass(ac, fn).run_pass() + # NOTE: MakeSSA is after algebraic optimization it currently produces + # smaller code by adding some redundant phi nodes. This is not a + # problem for us, but we need to be aware of it, and should be + # removed when the dft pass is fixed to produce the smallest code + # without making the code generation more expensive by running + # MakeSSA again. + MakeSSA(ac, fn).run_pass() BranchOptimizationPass(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() diff --git a/vyper/venom/analysis/cfg.py b/vyper/venom/analysis/cfg.py index e4f130bc18..90b18b353c 100644 --- a/vyper/venom/analysis/cfg.py +++ b/vyper/venom/analysis/cfg.py @@ -32,7 +32,10 @@ def analyze(self) -> None: in_bb.add_cfg_out(bb) def invalidate(self): - from vyper.venom.analysis import DominatorTreeAnalysis, LivenessAnalysis + from vyper.venom.analysis import DFGAnalysis, DominatorTreeAnalysis, LivenessAnalysis self.analyses_cache.invalidate_analysis(DominatorTreeAnalysis) self.analyses_cache.invalidate_analysis(LivenessAnalysis) + + # be conservative - assume cfg invalidation invalidates dfg + self.analyses_cache.invalidate_analysis(DFGAnalysis) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index 7966863081..19d373f81a 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -332,19 +332,18 @@ def _replace_constants(self, inst: IRInstruction): def _fix_phi_nodes(self): # fix basic blocks whose cfg in was changed # maybe this should really be done in _visit_phi - needs_sort = False - for bb in self.fn.get_basic_blocks(): cfg_in_labels = OrderedSet(in_bb.label for in_bb in bb.cfg_in) + needs_sort = False for inst in bb.instructions: if inst.opcode != "phi": break needs_sort |= self._fix_phi_inst(inst, cfg_in_labels) - # move phi instructions to the top of the block - if needs_sort: - bb.instructions.sort(key=lambda inst: inst.opcode != "phi") + # move phi instructions to the top of the block + if needs_sort: + bb.instructions.sort(key=lambda inst: inst.opcode != "phi") def _fix_phi_inst(self, inst: IRInstruction, cfg_in_labels: OrderedSet): operands = [op for label, op in inst.phi_operands if label in cfg_in_labels] diff --git a/vyper/venom/passes/store_elimination.py b/vyper/venom/passes/store_elimination.py index 0ecd324e26..559205adc8 100644 --- a/vyper/venom/passes/store_elimination.py +++ b/vyper/venom/passes/store_elimination.py @@ -27,16 +27,17 @@ def run_pass(self): self.analyses_cache.invalidate_analysis(LivenessAnalysis) self.analyses_cache.invalidate_analysis(DFGAnalysis) - def _process_store(self, dfg, inst, var, new_var): + def _process_store(self, dfg, inst, var: IRVariable, new_var: IRVariable): """ Process store instruction. If the variable is only used by a load instruction, forward the variable to the load instruction. """ - uses = dfg.get_uses(var) + if any([inst.opcode == "phi" for inst in dfg.get_uses(new_var)]): + return + uses = dfg.get_uses(var) if any([inst.opcode == "phi" for inst in uses]): return - for use_inst in uses: for i, operand in enumerate(use_inst.operands): if operand == var: