From 33ef4db55c327073cb66a0f8753d4187e7c527db Mon Sep 17 00:00:00 2001 From: Hodan Date: Wed, 4 Sep 2024 07:44:41 +0200 Subject: [PATCH 01/59] start cse --- vyper/venom/analysis/available_expression.py | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 vyper/venom/analysis/available_expression.py diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py new file mode 100644 index 0000000000..c00fb8218d --- /dev/null +++ b/vyper/venom/analysis/available_expression.py @@ -0,0 +1,5 @@ +from vyper.venom.analysis.analysis import IRAnalysis + +class AvailableExpressionAnalysis(IRAnalysis): + def analyze(self, *args, **kwargs): + return super().analyze(*args, **kwargs) From 52a34707e6c984f8dec9d3fe3d8677e87976f4a3 Mon Sep 17 00:00:00 2001 From: plodeada Date: Wed, 4 Sep 2024 15:30:03 +0200 Subject: [PATCH 02/59] cse start --- vyper/venom/__init__.py | 11 ++ vyper/venom/analysis/available_expression.py | 155 +++++++++++++++++- .../common_subexpression_elimination.py | 61 +++++++ 3 files changed, 226 insertions(+), 1 deletion(-) create mode 100644 vyper/venom/passes/common_subexpression_elimination.py diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index afd79fc44f..01784225ae 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -20,6 +20,7 @@ from vyper.venom.passes.simplify_cfg import SimplifyCFGPass from vyper.venom.passes.store_elimination import StoreElimination from vyper.venom.venom_to_assembly import VenomCompiler +from vyper.venom.passes.common_subexpression_elimination import CSE DEFAULT_OPT_LEVEL = OptimizationLevel.default() @@ -56,8 +57,18 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: BranchOptimizationPass(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() + CSE(ac, fn).run_pass() + RemoveUnusedVariablesPass(ac, fn).run_pass() DFTPass(ac, fn).run_pass() + #from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis + #avail: AvailableExpressionAnalysis = ac.force_analysis(AvailableExpressionAnalysis) + #for (bb, data) in avail.lattice.data.items(): + #print(bb.label) + #for (inst, varz) in data.data.items(): + #print("\t", inst, varz) + + def generate_ir(ir: IRnode, optimize: OptimizationLevel) -> IRContext: # Convert "old" IR to "new" IR diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index c00fb8218d..152e7247a0 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -1,5 +1,158 @@ from vyper.venom.analysis.analysis import IRAnalysis +from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.analysis.dfg import DFGAnalysis +from vyper.venom.analysis.cfg import CFGAnalysis +from vyper.venom.context import IRFunction +from vyper.venom.basicblock import IRBasicBlock +from vyper.venom.basicblock import IRInstruction +from vyper.venom.basicblock import IRLiteral +from vyper.venom.basicblock import IRVariable +from vyper.venom.basicblock import IROperand +from vyper.venom.basicblock import BB_TERMINATORS +from vyper.utils import OrderedSet +from dataclasses import dataclass + +@dataclass +class _Expression: + first_inst : IRInstruction + opcode: str + operands : list["_Expression | IROperand"] + + def __eq__(self, other): + if not isinstance(other, _Expression): + return False + return self.opcode == other.opcode and self.operands == other.operands + + def __hash__(self) -> int: + res : int = hash(self.opcode) + for op in self.operands: + res ^= hash(op) + return res + + def __repr__(self) -> str: + if self.opcode == "store": + assert len(self.operands) == 1 + return repr(self.operands[0]) + res = self.opcode + " [ " + for op in self.operands: + if self.opcode != "phi": + assert not isinstance(op, IRVariable) + res += repr(op) + " " + res += "]" + return res + + def contains_expr(self, expr : "_Expression") -> bool: + for op in self.operands: + if op == expr: + return True + if isinstance(op, _Expression) and op.contains_expr(expr): + return True + return False + +class _BBLattice: + data : dict[IRInstruction, OrderedSet[_Expression]] + out : OrderedSet[_Expression] + + def __init__(self, bb : IRBasicBlock): + self.data = dict() + self.out = OrderedSet() + for inst in bb.instructions: + self.data[inst] = OrderedSet() + +UNINTRESTING_OPCODES = [ + "store", + "param", + "offset", + "phi", + "nop", + "assert", +] + +class _FunctionLattice: + data : dict[IRBasicBlock, _BBLattice] + + def __init__(self, function : IRFunction): + self.data = dict() + for bb in function.get_basic_blocks(): + self.data[bb] = _BBLattice(bb) class AvailableExpressionAnalysis(IRAnalysis): + expressions : OrderedSet[_Expression] = OrderedSet() + inst_to_expr: dict[IRInstruction, _Expression] = dict() + dfg: DFGAnalysis + lattice : _FunctionLattice + + def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): + super().__init__(analyses_cache, function) + self.analyses_cache.request_analysis(CFGAnalysis) + dfg = self.analyses_cache.request_analysis(DFGAnalysis) + assert isinstance(dfg, DFGAnalysis) + self.dfg = dfg + + self.lattice = _FunctionLattice(function) + def analyze(self, *args, **kwargs): - return super().analyze(*args, **kwargs) + while True: + changed = False + for bb in self.function.get_basic_blocks(): + changed |= self._handle_bb(bb) + + if not changed: + break + + def _handle_bb(self, bb : IRBasicBlock) -> bool: + available_expr : OrderedSet[_Expression] = OrderedSet() + if len(bb.cfg_in) > 0: + available_expr = self.lattice.data[bb.cfg_in.first()].out + for in_bb in bb.cfg_in: + available_expr = available_expr.union(self.lattice.data[in_bb].out) + + bb_lat = self.lattice.data[bb] + change = False + for inst in bb.instructions: + if inst == "phi": + continue + inst_expr = self.get_expression(inst) + if available_expr != bb_lat.data[inst]: + bb_lat.data[inst] = available_expr.copy() + change |= True + if inst_expr.opcode not in UNINTRESTING_OPCODES and inst_expr.opcode not in BB_TERMINATORS and "call" not in inst_expr.opcode and inst.output != None: + for expr in available_expr.copy(): + if expr.contains_expr(inst_expr): + available_expr.remove(expr) + available_expr.add(inst_expr) + + if available_expr != bb_lat.out: + bb_lat.out = available_expr.copy() + change |= True + + return change + + def _get_operand(self, op : IROperand) -> _Expression | IROperand: + if isinstance(op, IRVariable): + inst = self.dfg.get_producing_instruction(op) + assert inst is not None + return self.get_expression(inst) + return op + + def get_expression(self, inst: IRInstruction) -> _Expression: + if inst in self.inst_to_expr.keys(): + return self.inst_to_expr[inst] + if inst.opcode == "phi": + operands: list[_Expression | IROperand] = inst.operands.copy() + else: + operands = [self._get_operand(op) for op in inst.operands] + expr = _Expression(inst, inst.opcode, operands) + for e in self.expressions: + if e == expr: + self.inst_to_expr[inst] = e + #print("yo", e) + return e + + self.expressions.add(expr) + self.inst_to_expr[inst] = expr + return expr + + def get_available(self, inst : IRInstruction) -> OrderedSet[_Expression]: + return self.lattice.data[inst.parent].data[inst] + diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py new file mode 100644 index 0000000000..f5c5009522 --- /dev/null +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -0,0 +1,61 @@ +from vyper.venom.passes.base_pass import IRPass +from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis +from vyper.venom.analysis.dfg import DFGAnalysis +from vyper.venom.basicblock import IRInstruction +from vyper.venom.basicblock import IRVariable +from vyper.venom.basicblock import IRBasicBlock +from vyper.utils import OrderedSet + +class CSE(IRPass): + available_expression_analysis : AvailableExpressionAnalysis + + def run_pass(self, *args, **kwargs): + available_expression_analysis = self.analyses_cache.request_analysis(AvailableExpressionAnalysis) + assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) + self.available_expression_analysis = available_expression_analysis + + while True: + replace_dict = self._find_replaceble() + if len(replace_dict) == 0: + break + self._replace(replace_dict) + self.analyses_cache.invalidate_analysis(DFGAnalysis) + + # return instruction and to which instruction it could + # replaced by + def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: + res : dict[IRInstruction, IRInstruction] = dict() + for bb in self.function.get_basic_blocks(): + for inst in bb.instructions: + inst_expr = self.available_expression_analysis.get_expression(inst) + avail = self.available_expression_analysis.get_available(inst) + if inst_expr in avail: + #print(inst, inst_expr) + res[inst] = inst_expr.first_inst + + return res + + def _replace(self, replace_dict : dict[IRInstruction, IRInstruction]): + for (orig, to) in replace_dict.items(): + self._replace_inst(orig, to) + + def _replace_inst(self, orig_inst : IRInstruction, to_inst : IRInstruction): + visited: OrderedSet[IRBasicBlock] = OrderedSet() + assert isinstance(orig_inst.output, IRVariable), f"not var {orig_inst}" + assert isinstance(to_inst.output, IRVariable), "not var" + self._replace_inst_r(orig_inst.parent, orig_inst.output, to_inst.output, visited) + orig_inst.parent.remove_instruction(orig_inst) + + def _replace_inst_r(self, bb : IRBasicBlock, orig : IRVariable, to : IRVariable, visited : OrderedSet[IRBasicBlock]): + if bb in visited: + return + visited.add(bb) + + for inst in bb.instructions: + for i in range(len(inst.operands)): + op = inst.operands[i] + if op == orig: + inst.operands[i] = to + + for out in bb.cfg_out: + self._replace_inst_r(out, orig, to, visited) From c2d60edc88494e1164a63194089bf8e7896dfa2c Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 9 Sep 2024 09:05:34 +0200 Subject: [PATCH 03/59] only one inst handling --- vyper/venom/__init__.py | 27 +++++--- vyper/venom/analysis/available_expression.py | 69 +++++++++++++------ .../common_subexpression_elimination.py | 18 +++-- 3 files changed, 79 insertions(+), 35 deletions(-) diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 01784225ae..b032fcc421 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -55,18 +55,29 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: SimplifyCFGPass(ac, fn).run_pass() AlgebraicOptimizationPass(ac, fn).run_pass() BranchOptimizationPass(ac, fn).run_pass() - ExtractLiteralsPass(ac, fn).run_pass() - RemoveUnusedVariablesPass(ac, fn).run_pass() CSE(ac, fn).run_pass() + ExtractLiteralsPass(ac, fn).run_pass() + #RemoveUnusedVariablesPass(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() + #print(fn) DFTPass(ac, fn).run_pass() - #from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis - #avail: AvailableExpressionAnalysis = ac.force_analysis(AvailableExpressionAnalysis) - #for (bb, data) in avail.lattice.data.items(): - #print(bb.label) - #for (inst, varz) in data.data.items(): - #print("\t", inst, varz) + """ + from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis + avail: AvailableExpressionAnalysis = ac.force_analysis(AvailableExpressionAnalysis) + for (bb, data) in avail.lattice.data.items(): + print(bb.label, "out", data.out) + for (inst, varz) in data.data.items(): + print("\t", inst, varz) + """ + + from vyper.venom.analysis.liveness import LivenessAnalysis + print("########") + print("########") + print("########") + print("########") + print("########") + ac.force_analysis(LivenessAnalysis) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 152e7247a0..38b16e92db 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -11,6 +11,7 @@ from vyper.venom.basicblock import BB_TERMINATORS from vyper.utils import OrderedSet from dataclasses import dataclass +from collections import deque @dataclass class _Expression: @@ -31,12 +32,12 @@ def __hash__(self) -> int: def __repr__(self) -> str: if self.opcode == "store": - assert len(self.operands) == 1 + assert len(self.operands) == 1, "wrong store" return repr(self.operands[0]) res = self.opcode + " [ " for op in self.operands: - if self.opcode != "phi": - assert not isinstance(op, IRVariable) + #if self.opcode != "phi": + #assert not isinstance(op, IRVariable) res += repr(op) + " " res += "]" return res @@ -52,10 +53,12 @@ def contains_expr(self, expr : "_Expression") -> bool: class _BBLattice: data : dict[IRInstruction, OrderedSet[_Expression]] out : OrderedSet[_Expression] + in_cache: OrderedSet[_Expression] | None def __init__(self, bb : IRBasicBlock): self.data = dict() self.out = OrderedSet() + self.in_cache = None for inst in bb.instructions: self.data[inst] = OrderedSet() @@ -92,44 +95,68 @@ def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): self.lattice = _FunctionLattice(function) def analyze(self, *args, **kwargs): - while True: - changed = False - for bb in self.function.get_basic_blocks(): - changed |= self._handle_bb(bb) - - if not changed: - break + worklist = deque() + worklist.append(self.function.entry) + while len(worklist) > 0: + bb : IRBasicBlock = worklist.popleft() + changed = self._handle_bb(bb) + + if changed: + for out in bb.cfg_out: + if out not in worklist: + worklist.append(out) def _handle_bb(self, bb : IRBasicBlock) -> bool: available_expr : OrderedSet[_Expression] = OrderedSet() if len(bb.cfg_in) > 0: - available_expr = self.lattice.data[bb.cfg_in.first()].out - for in_bb in bb.cfg_in: - available_expr = available_expr.union(self.lattice.data[in_bb].out) + available_expr = OrderedSet.intersection(*(self.lattice.data[in_bb].out for in_bb in bb.cfg_in)) + if bb.label.name == "5_if_exit": + pass + print(bb.label) + print(type(available_expr)) + print(type(available_expr._data)) + print(len(available_expr)) + print(available_expr) + bb_lat = self.lattice.data[bb] + if bb_lat.in_cache is not None and available_expr == bb_lat.in_cache: + return False + bb_lat.in_cache = available_expr change = False for inst in bb.instructions: - if inst == "phi": + if bb.label.name == "5_if_exit": + print(inst) + if "call" in inst.opcode: + for expr in available_expr.copy(): + if "returndata" in expr.opcode: + available_expr.remove(expr) + continue + if (inst.opcode in UNINTRESTING_OPCODES + or inst.opcode in BB_TERMINATORS + or inst.output == None): continue inst_expr = self.get_expression(inst) + if inst.output is not None and inst.output.name == "%25": + print(available_expr) if available_expr != bb_lat.data[inst]: bb_lat.data[inst] = available_expr.copy() change |= True - if inst_expr.opcode not in UNINTRESTING_OPCODES and inst_expr.opcode not in BB_TERMINATORS and "call" not in inst_expr.opcode and inst.output != None: - for expr in available_expr.copy(): - if expr.contains_expr(inst_expr): - available_expr.remove(expr) - available_expr.add(inst_expr) + for expr in available_expr.copy(): + if expr.contains_expr(inst_expr): + available_expr.remove(expr) + available_expr.add(inst_expr) if available_expr != bb_lat.out: bb_lat.out = available_expr.copy() change |= True - + + #if change: + #print(bb.label) return change def _get_operand(self, op : IROperand) -> _Expression | IROperand: - if isinstance(op, IRVariable): + if False and isinstance(op, IRVariable): inst = self.dfg.get_producing_instruction(op) assert inst is not None return self.get_expression(inst) diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index f5c5009522..bdcc7b80dd 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -1,5 +1,6 @@ from vyper.venom.passes.base_pass import IRPass from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis +from vyper.venom.analysis.liveness import LivenessAnalysis from vyper.venom.analysis.dfg import DFGAnalysis from vyper.venom.basicblock import IRInstruction from vyper.venom.basicblock import IRVariable @@ -14,12 +15,12 @@ def run_pass(self, *args, **kwargs): assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) self.available_expression_analysis = available_expression_analysis - while True: - replace_dict = self._find_replaceble() - if len(replace_dict) == 0: - break - self._replace(replace_dict) - self.analyses_cache.invalidate_analysis(DFGAnalysis) + replace_dict = self._find_replaceble() + if len(replace_dict) == 0: + return + self._replace(replace_dict) + self.analyses_cache.invalidate_analysis(DFGAnalysis) + self.analyses_cache.invalidate_analysis(LivenessAnalysis) # return instruction and to which instruction it could # replaced by @@ -27,6 +28,7 @@ def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: res : dict[IRInstruction, IRInstruction] = dict() for bb in self.function.get_basic_blocks(): for inst in bb.instructions: + #print(inst) inst_expr = self.available_expression_analysis.get_expression(inst) avail = self.available_expression_analysis.get_available(inst) if inst_expr in avail: @@ -37,6 +39,7 @@ def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: def _replace(self, replace_dict : dict[IRInstruction, IRInstruction]): for (orig, to) in replace_dict.items(): + print(orig.output, to.output) self._replace_inst(orig, to) def _replace_inst(self, orig_inst : IRInstruction, to_inst : IRInstruction): @@ -52,10 +55,13 @@ def _replace_inst_r(self, bb : IRBasicBlock, orig : IRVariable, to : IRVariable, visited.add(bb) for inst in bb.instructions: + #print("\t", inst) for i in range(len(inst.operands)): op = inst.operands[i] if op == orig: + #print("yo") inst.operands[i] = to + #print(inst) for out in bb.cfg_out: self._replace_inst_r(out, orig, to, visited) From 3087650d1820b2be4eb329a1821a8723196a2073 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 10 Sep 2024 09:58:26 +0200 Subject: [PATCH 04/59] cleanup + fix --- vyper/venom/__init__.py | 17 ------- vyper/venom/analysis/available_expression.py | 49 ++++--------------- .../common_subexpression_elimination.py | 7 ++- 3 files changed, 13 insertions(+), 60 deletions(-) diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index b032fcc421..29c2b6c399 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -62,23 +62,6 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: #print(fn) DFTPass(ac, fn).run_pass() - """ - from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis - avail: AvailableExpressionAnalysis = ac.force_analysis(AvailableExpressionAnalysis) - for (bb, data) in avail.lattice.data.items(): - print(bb.label, "out", data.out) - for (inst, varz) in data.data.items(): - print("\t", inst, varz) - """ - - from vyper.venom.analysis.liveness import LivenessAnalysis - print("########") - print("########") - print("########") - print("########") - print("########") - ac.force_analysis(LivenessAnalysis) - def generate_ir(ir: IRnode, optimize: OptimizationLevel) -> IRContext: diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 38b16e92db..474c1b97ab 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -17,7 +17,7 @@ class _Expression: first_inst : IRInstruction opcode: str - operands : list["_Expression | IROperand"] + operands : list[IROperand] def __eq__(self, other): if not isinstance(other, _Expression): @@ -36,8 +36,6 @@ def __repr__(self) -> str: return repr(self.operands[0]) res = self.opcode + " [ " for op in self.operands: - #if self.opcode != "phi": - #assert not isinstance(op, IRVariable) res += repr(op) + " " res += "]" return res @@ -111,22 +109,12 @@ def _handle_bb(self, bb : IRBasicBlock) -> bool: if len(bb.cfg_in) > 0: available_expr = OrderedSet.intersection(*(self.lattice.data[in_bb].out for in_bb in bb.cfg_in)) - if bb.label.name == "5_if_exit": - pass - print(bb.label) - print(type(available_expr)) - print(type(available_expr._data)) - print(len(available_expr)) - print(available_expr) - bb_lat = self.lattice.data[bb] if bb_lat.in_cache is not None and available_expr == bb_lat.in_cache: return False bb_lat.in_cache = available_expr change = False for inst in bb.instructions: - if bb.label.name == "5_if_exit": - print(inst) if "call" in inst.opcode: for expr in available_expr.copy(): if "returndata" in expr.opcode: @@ -136,9 +124,7 @@ def _handle_bb(self, bb : IRBasicBlock) -> bool: or inst.opcode in BB_TERMINATORS or inst.output == None): continue - inst_expr = self.get_expression(inst) - if inst.output is not None and inst.output.name == "%25": - print(available_expr) + inst_expr = self.get_expression(inst, available_expr) if available_expr != bb_lat.data[inst]: bb_lat.data[inst] = available_expr.copy() change |= True @@ -151,33 +137,18 @@ def _handle_bb(self, bb : IRBasicBlock) -> bool: bb_lat.out = available_expr.copy() change |= True - #if change: - #print(bb.label) return change - def _get_operand(self, op : IROperand) -> _Expression | IROperand: - if False and isinstance(op, IRVariable): - inst = self.dfg.get_producing_instruction(op) - assert inst is not None - return self.get_expression(inst) - return op - - def get_expression(self, inst: IRInstruction) -> _Expression: - if inst in self.inst_to_expr.keys(): - return self.inst_to_expr[inst] - if inst.opcode == "phi": - operands: list[_Expression | IROperand] = inst.operands.copy() - else: - operands = [self._get_operand(op) for op in inst.operands] + def get_expression(self, inst: IRInstruction, available_exprs : OrderedSet[_Expression] | None = None) -> _Expression: + if available_exprs is None: + available_exprs = self.lattice.data[inst.parent].data[inst] + operands: list[IROperand] = inst.operands.copy() expr = _Expression(inst, inst.opcode, operands) - for e in self.expressions: - if e == expr: - self.inst_to_expr[inst] = e - #print("yo", e) - return e + if expr in available_exprs: + for e in available_exprs: + if e == expr: + return e - self.expressions.add(expr) - self.inst_to_expr[inst] = expr return expr def get_available(self, inst : IRInstruction) -> OrderedSet[_Expression]: diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index bdcc7b80dd..11ed488e94 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -39,7 +39,9 @@ def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: def _replace(self, replace_dict : dict[IRInstruction, IRInstruction]): for (orig, to) in replace_dict.items(): - print(orig.output, to.output) + while to in replace_dict.keys(): + to = replace_dict[to] + #print(orig.output, to.output) self._replace_inst(orig, to) def _replace_inst(self, orig_inst : IRInstruction, to_inst : IRInstruction): @@ -55,13 +57,10 @@ def _replace_inst_r(self, bb : IRBasicBlock, orig : IRVariable, to : IRVariable, visited.add(bb) for inst in bb.instructions: - #print("\t", inst) for i in range(len(inst.operands)): op = inst.operands[i] if op == orig: - #print("yo") inst.operands[i] = to - #print(inst) for out in bb.cfg_out: self._replace_inst_r(out, orig, to, visited) From bad3f693164de671625bbffe1183b6d3ee675853 Mon Sep 17 00:00:00 2001 From: Hodan Date: Thu, 12 Sep 2024 16:07:22 +0200 Subject: [PATCH 05/59] effects start --- vyper/venom/analysis/available_expression.py | 51 +++++++++++++++++-- .../common_subexpression_elimination.py | 2 - vyper/venom/venom_to_assembly.py | 2 + 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 474c1b97ab..52673f91ae 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -69,6 +69,46 @@ def __init__(self, bb : IRBasicBlock): "assert", ] +_ALL = ("storage", "transient", "memory", "immutables", "balance", "returndata") + +writes = { + "sstore": ("storage"), + "tstore": ("transient"), + "mstore": ("memory"), + "istore": ("immutables"), + "call": _ALL, + "delegatecall": _ALL, + "staticcall": ("memory"), + "create": _ALL, + "create2": _ALL, + "invoke": _ALL, # could be smarter, look up the effects of the invoked function + "dloadbytes": ("memory"), + "returndatacopy": ("memory"), + "calldatacopy": ("memory"), + "codecopy": ("memory"), + "extcodecopy": ("memory"), + "mcopy": ("memory"), +} +reads = { + "sload": ("storage"), + "tload": ("transient"), + "iload": ("immutables"), + "mload": ("memory"), + "mcopy": ("memory"), + "call": _ALL, + "delegatecall": _ALL, + "staticcall": _ALL, + "returndatasize": ("returndata"), + "returndatacopy": ("returndata"), + "balance": ("balance"), + "selfbalance": ("balance"), + "log": ("memory"), + "revert": ("memory"), + "return": ("memory"), + "sha3": ("memory"), +} + + class _FunctionLattice: data : dict[IRBasicBlock, _BBLattice] @@ -115,11 +155,6 @@ def _handle_bb(self, bb : IRBasicBlock) -> bool: bb_lat.in_cache = available_expr change = False for inst in bb.instructions: - if "call" in inst.opcode: - for expr in available_expr.copy(): - if "returndata" in expr.opcode: - available_expr.remove(expr) - continue if (inst.opcode in UNINTRESTING_OPCODES or inst.opcode in BB_TERMINATORS or inst.output == None): @@ -128,9 +163,15 @@ def _handle_bb(self, bb : IRBasicBlock) -> bool: if available_expr != bb_lat.data[inst]: bb_lat.data[inst] = available_expr.copy() change |= True + + write_effects = writes.get(inst_expr.opcode, ()) for expr in available_expr.copy(): if expr.contains_expr(inst_expr): available_expr.remove(expr) + read_effects = reads.get(expr.opcode, ()) + if any(eff in write_effects for eff in read_effects): + available_expr.remove(expr) + available_expr.add(inst_expr) if available_expr != bb_lat.out: diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 11ed488e94..744d8451e7 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -28,11 +28,9 @@ def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: res : dict[IRInstruction, IRInstruction] = dict() for bb in self.function.get_basic_blocks(): for inst in bb.instructions: - #print(inst) inst_expr = self.available_expression_analysis.get_expression(inst) avail = self.available_expression_analysis.get_available(inst) if inst_expr in avail: - #print(inst, inst_expr) res[inst] = inst_expr.first_inst return res diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 07d63afc70..95aa70b300 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -156,6 +156,7 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: ac.request_analysis(DupRequirementsAnalysis) assert fn.normalized, "Non-normalized CFG!" + print(fn.name) self._generate_evm_for_basicblock_r(asm, fn.entry, StackModel()) @@ -347,6 +348,7 @@ def clean_stack_from_cfg_in( def _generate_evm_for_instruction( self, inst: IRInstruction, stack: StackModel, next_liveness: OrderedSet = None ) -> list[str]: + print("\t", inst) assembly: list[str | int] = [] next_liveness = next_liveness or OrderedSet() opcode = inst.opcode From 9798a39ff02cea471a00221f2aa883aac35b07cb Mon Sep 17 00:00:00 2001 From: plodeada Date: Fri, 13 Sep 2024 12:29:44 +0200 Subject: [PATCH 06/59] correct compare for joins --- vyper/venom/__init__.py | 4 +++- vyper/venom/analysis/available_expression.py | 11 ++++++----- vyper/venom/venom_to_assembly.py | 4 ++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 29c2b6c399..cafcc31f84 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -57,11 +57,13 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: BranchOptimizationPass(ac, fn).run_pass() CSE(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() - #RemoveUnusedVariablesPass(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() #print(fn) DFTPass(ac, fn).run_pass() + from vyper.venom.analysis.liveness import LivenessAnalysis + ac.force_analysis(LivenessAnalysis) + def generate_ir(ir: IRnode, optimize: OptimizationLevel) -> IRContext: diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 52673f91ae..7e5badac13 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -22,7 +22,8 @@ class _Expression: def __eq__(self, other): if not isinstance(other, _Expression): return False - return self.opcode == other.opcode and self.operands == other.operands + #return self.opcode == other.opcode and self.operands == other.operands and fi + return self.first_inst == other.first_inst def __hash__(self) -> int: res : int = hash(self.opcode) @@ -185,10 +186,10 @@ def get_expression(self, inst: IRInstruction, available_exprs : OrderedSet[_Expr available_exprs = self.lattice.data[inst.parent].data[inst] operands: list[IROperand] = inst.operands.copy() expr = _Expression(inst, inst.opcode, operands) - if expr in available_exprs: - for e in available_exprs: - if e == expr: - return e + #if expr in available_exprs: + for e in available_exprs: + if e.opcode == expr.opcode and e.operands == expr.operands: + return e return expr diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 95aa70b300..ffebadb2a4 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -156,7 +156,7 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: ac.request_analysis(DupRequirementsAnalysis) assert fn.normalized, "Non-normalized CFG!" - print(fn.name) + #print(fn.name) self._generate_evm_for_basicblock_r(asm, fn.entry, StackModel()) @@ -348,7 +348,7 @@ def clean_stack_from_cfg_in( def _generate_evm_for_instruction( self, inst: IRInstruction, stack: StackModel, next_liveness: OrderedSet = None ) -> list[str]: - print("\t", inst) + #print("\t", inst, stack) assembly: list[str | int] = [] next_liveness = next_liveness or OrderedSet() opcode = inst.opcode From ea92aa08473cd8c7e2ee3f49b447494817a233bb Mon Sep 17 00:00:00 2001 From: plodeada Date: Fri, 13 Sep 2024 14:03:03 +0200 Subject: [PATCH 07/59] fix for calls and instruction without output (i.e. stores) --- vyper/venom/analysis/available_expression.py | 9 +++++---- vyper/venom/passes/common_subexpression_elimination.py | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 7e5badac13..bfcbc71a2f 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -157,14 +157,14 @@ def _handle_bb(self, bb : IRBasicBlock) -> bool: change = False for inst in bb.instructions: if (inst.opcode in UNINTRESTING_OPCODES - or inst.opcode in BB_TERMINATORS - or inst.output == None): + or inst.opcode in BB_TERMINATORS): continue - inst_expr = self.get_expression(inst, available_expr) if available_expr != bb_lat.data[inst]: bb_lat.data[inst] = available_expr.copy() change |= True + + inst_expr = self.get_expression(inst, available_expr) write_effects = writes.get(inst_expr.opcode, ()) for expr in available_expr.copy(): if expr.contains_expr(inst_expr): @@ -173,7 +173,8 @@ def _handle_bb(self, bb : IRBasicBlock) -> bool: if any(eff in write_effects for eff in read_effects): available_expr.remove(expr) - available_expr.add(inst_expr) + if "call" not in inst.opcode and inst.opcode != "invoke": + available_expr.add(inst_expr) if available_expr != bb_lat.out: bb_lat.out = available_expr.copy() diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 744d8451e7..e4b98b8719 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -44,9 +44,10 @@ def _replace(self, replace_dict : dict[IRInstruction, IRInstruction]): def _replace_inst(self, orig_inst : IRInstruction, to_inst : IRInstruction): visited: OrderedSet[IRBasicBlock] = OrderedSet() - assert isinstance(orig_inst.output, IRVariable), f"not var {orig_inst}" - assert isinstance(to_inst.output, IRVariable), "not var" - self._replace_inst_r(orig_inst.parent, orig_inst.output, to_inst.output, visited) + if orig_inst.output is not None: + assert isinstance(orig_inst.output, IRVariable), f"not var {orig_inst}" + assert isinstance(to_inst.output, IRVariable), f"not var {to_inst}" + self._replace_inst_r(orig_inst.parent, orig_inst.output, to_inst.output, visited) orig_inst.parent.remove_instruction(orig_inst) def _replace_inst_r(self, bb : IRBasicBlock, orig : IRVariable, to : IRVariable, visited : OrderedSet[IRBasicBlock]): From c754574eba972e5eeef5aa30022cefd8f2ea5d81 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 16 Sep 2024 10:10:28 +0200 Subject: [PATCH 08/59] small cleanup --- vyper/venom/analysis/available_expression.py | 7 ++----- vyper/venom/passes/common_subexpression_elimination.py | 1 - 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index bfcbc71a2f..f79eb8fbba 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -22,7 +22,6 @@ class _Expression: def __eq__(self, other): if not isinstance(other, _Expression): return False - #return self.opcode == other.opcode and self.operands == other.operands and fi return self.first_inst == other.first_inst def __hash__(self) -> int: @@ -61,13 +60,12 @@ def __init__(self, bb : IRBasicBlock): for inst in bb.instructions: self.data[inst] = OrderedSet() -UNINTRESTING_OPCODES = [ +_UNINTRESTING_OPCODES = [ "store", "param", "offset", "phi", "nop", - "assert", ] _ALL = ("storage", "transient", "memory", "immutables", "balance", "returndata") @@ -156,7 +154,7 @@ def _handle_bb(self, bb : IRBasicBlock) -> bool: bb_lat.in_cache = available_expr change = False for inst in bb.instructions: - if (inst.opcode in UNINTRESTING_OPCODES + if (inst.opcode in _UNINTRESTING_OPCODES or inst.opcode in BB_TERMINATORS): continue if available_expr != bb_lat.data[inst]: @@ -187,7 +185,6 @@ def get_expression(self, inst: IRInstruction, available_exprs : OrderedSet[_Expr available_exprs = self.lattice.data[inst.parent].data[inst] operands: list[IROperand] = inst.operands.copy() expr = _Expression(inst, inst.opcode, operands) - #if expr in available_exprs: for e in available_exprs: if e.opcode == expr.opcode and e.operands == expr.operands: return e diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index e4b98b8719..7942442cc1 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -39,7 +39,6 @@ def _replace(self, replace_dict : dict[IRInstruction, IRInstruction]): for (orig, to) in replace_dict.items(): while to in replace_dict.keys(): to = replace_dict[to] - #print(orig.output, to.output) self._replace_inst(orig, to) def _replace_inst(self, orig_inst : IRInstruction, to_inst : IRInstruction): From e6a85515013f25ec0ddb20483290d097bc90b7d5 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 16 Sep 2024 10:21:07 +0200 Subject: [PATCH 09/59] small cleanup --- vyper/venom/__init__.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index cafcc31f84..206da01085 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -58,13 +58,8 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: CSE(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() - #print(fn) DFTPass(ac, fn).run_pass() - from vyper.venom.analysis.liveness import LivenessAnalysis - ac.force_analysis(LivenessAnalysis) - - def generate_ir(ir: IRnode, optimize: OptimizationLevel) -> IRContext: # Convert "old" IR to "new" IR From 6136f83ebbf346a3ab506115054885cd71eec19d Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 16 Sep 2024 11:28:09 +0200 Subject: [PATCH 10/59] basic test created and fix small fix --- .../test_common_subexpression_elimination.py | 23 +++++++++++++++++++ .../common_subexpression_elimination.py | 15 ++++++------ 2 files changed, 31 insertions(+), 7 deletions(-) create mode 100644 tests/unit/compiler/venom/test_common_subexpression_elimination.py diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py new file mode 100644 index 0000000000..99cdd4c7fc --- /dev/null +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -0,0 +1,23 @@ +from vyper.venom.context import IRContext +from vyper.venom.passes.common_subexpression_elimination import CSE +from vyper.venom.passes.extract_literals import ExtractLiteralsPass +from vyper.venom.analysis.analysis import IRAnalysesCache + +def test_common_subexpression_elimination(): + ctx = IRContext() + fn = ctx.create_function("test") + bb = fn.get_basic_block() + op = bb.append_instruction("store", 10) + sum_1 = bb.append_instruction("add", op, 10) + bb.append_instruction("mul", sum_1, 10) + sum_2 = bb.append_instruction("add", op, 10) + bb.append_instruction("mul", sum_2, 10) + bb.append_instruction("stop") + + + ac = IRAnalysesCache(fn) + CSE(ac, fn).run_pass() + ExtractLiteralsPass(ac, fn).run_pass() + + assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 1, "wrong number of adds" + assert sum(1 for inst in bb.instructions if inst.opcode == "mul") == 1, "wrong number of muls" diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 7942442cc1..a03d2fa42f 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -14,13 +14,14 @@ def run_pass(self, *args, **kwargs): available_expression_analysis = self.analyses_cache.request_analysis(AvailableExpressionAnalysis) assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) self.available_expression_analysis = available_expression_analysis - - replace_dict = self._find_replaceble() - if len(replace_dict) == 0: - return - self._replace(replace_dict) - self.analyses_cache.invalidate_analysis(DFGAnalysis) - self.analyses_cache.invalidate_analysis(LivenessAnalysis) + + while True: + replace_dict = self._find_replaceble() + if len(replace_dict) == 0: + return + self._replace(replace_dict) + self.analyses_cache.invalidate_analysis(DFGAnalysis) + self.analyses_cache.invalidate_analysis(LivenessAnalysis) # return instruction and to which instruction it could # replaced by From 2d7e1a4c800560ade08b912dc7cdfc53ea86a8c4 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 16 Sep 2024 13:39:26 +0200 Subject: [PATCH 11/59] clean up of the debug --- vyper/venom/venom_to_assembly.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index ffebadb2a4..07d63afc70 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -156,7 +156,6 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: ac.request_analysis(DupRequirementsAnalysis) assert fn.normalized, "Non-normalized CFG!" - #print(fn.name) self._generate_evm_for_basicblock_r(asm, fn.entry, StackModel()) @@ -348,7 +347,6 @@ def clean_stack_from_cfg_in( def _generate_evm_for_instruction( self, inst: IRInstruction, stack: StackModel, next_liveness: OrderedSet = None ) -> list[str]: - #print("\t", inst, stack) assembly: list[str | int] = [] next_liveness = next_liveness or OrderedSet() opcode = inst.opcode From 4b6295838344c582f0068881833077e8edc7f7bf Mon Sep 17 00:00:00 2001 From: HodanPlodky <36966616+HodanPlodky@users.noreply.github.com> Date: Mon, 16 Sep 2024 18:56:42 +0000 Subject: [PATCH 12/59] Update vyper/venom/analysis/available_expression.py - better hash method Co-authored-by: Charles Cooper --- vyper/venom/analysis/available_expression.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index f79eb8fbba..8cf341f0bb 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -25,10 +25,7 @@ def __eq__(self, other): return self.first_inst == other.first_inst def __hash__(self) -> int: - res : int = hash(self.opcode) - for op in self.operands: - res ^= hash(op) - return res + return hash((self.opcode, *self.operands)) def __repr__(self) -> str: if self.opcode == "store": From fbf2964ac76badea9ffd6b252cd8cf14304a53ce Mon Sep 17 00:00:00 2001 From: HodanPlodky <36966616+HodanPlodky@users.noreply.github.com> Date: Mon, 16 Sep 2024 18:57:06 +0000 Subject: [PATCH 13/59] Update vyper/venom/analysis/available_expression.py - Incorrect grammar fix Co-authored-by: Charles Cooper --- vyper/venom/analysis/available_expression.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 8cf341f0bb..d96792276a 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -57,7 +57,7 @@ def __init__(self, bb : IRBasicBlock): for inst in bb.instructions: self.data[inst] = OrderedSet() -_UNINTRESTING_OPCODES = [ +_UNINTERESTING_OPCODES = [ "store", "param", "offset", From 86977f58e14a7c3a6bfea68785dc8f9e92fe3208 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 17 Sep 2024 16:18:22 +0200 Subject: [PATCH 14/59] fix for some error in cancun version of experimental codegen test --- vyper/venom/__init__.py | 1 + vyper/venom/analysis/available_expression.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 206da01085..3dd3fbd9ad 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -55,6 +55,7 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: SimplifyCFGPass(ac, fn).run_pass() AlgebraicOptimizationPass(ac, fn).run_pass() BranchOptimizationPass(ac, fn).run_pass() + RemoveUnusedVariablesPass(ac, fn).run_pass() CSE(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index d96792276a..bb722162cd 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -151,7 +151,7 @@ def _handle_bb(self, bb : IRBasicBlock) -> bool: bb_lat.in_cache = available_expr change = False for inst in bb.instructions: - if (inst.opcode in _UNINTRESTING_OPCODES + if (inst.opcode in _UNINTERESTING_OPCODES or inst.opcode in BB_TERMINATORS): continue if available_expr != bb_lat.data[inst]: From 49e6949e573459f7b8e740a8efc03b0410928d2a Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 17 Sep 2024 17:03:29 +0200 Subject: [PATCH 15/59] fix for log opcode and lint --- .../test_common_subexpression_elimination.py | 4 +- vyper/venom/__init__.py | 2 +- vyper/venom/analysis/available_expression.py | 84 +++++++++---------- .../common_subexpression_elimination.py | 33 ++++---- 4 files changed, 60 insertions(+), 63 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 99cdd4c7fc..d72218ae7f 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -1,7 +1,8 @@ +from vyper.venom.analysis.analysis import IRAnalysesCache from vyper.venom.context import IRContext from vyper.venom.passes.common_subexpression_elimination import CSE from vyper.venom.passes.extract_literals import ExtractLiteralsPass -from vyper.venom.analysis.analysis import IRAnalysesCache + def test_common_subexpression_elimination(): ctx = IRContext() @@ -13,7 +14,6 @@ def test_common_subexpression_elimination(): sum_2 = bb.append_instruction("add", op, 10) bb.append_instruction("mul", sum_2, 10) bb.append_instruction("stop") - ac = IRAnalysesCache(fn) CSE(ac, fn).run_pass() diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 3dd3fbd9ad..0bbae0a402 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -11,6 +11,7 @@ from vyper.venom.ir_node_to_venom import ir_node_to_venom from vyper.venom.passes.algebraic_optimization import AlgebraicOptimizationPass from vyper.venom.passes.branch_optimization import BranchOptimizationPass +from vyper.venom.passes.common_subexpression_elimination import CSE from vyper.venom.passes.dft import DFTPass from vyper.venom.passes.extract_literals import ExtractLiteralsPass from vyper.venom.passes.make_ssa import MakeSSA @@ -20,7 +21,6 @@ from vyper.venom.passes.simplify_cfg import SimplifyCFGPass from vyper.venom.passes.store_elimination import StoreElimination from vyper.venom.venom_to_assembly import VenomCompiler -from vyper.venom.passes.common_subexpression_elimination import CSE DEFAULT_OPT_LEVEL = OptimizationLevel.default() diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index bb722162cd..d0486f253f 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -1,29 +1,25 @@ -from vyper.venom.analysis.analysis import IRAnalysis -from vyper.venom.analysis.analysis import IRAnalysesCache -from vyper.venom.analysis.dfg import DFGAnalysis +from collections import deque +from dataclasses import dataclass + +from vyper.utils import OrderedSet +from vyper.venom.analysis.analysis import IRAnalysesCache, IRAnalysis from vyper.venom.analysis.cfg import CFGAnalysis +from vyper.venom.analysis.dfg import DFGAnalysis +from vyper.venom.basicblock import BB_TERMINATORS, IRBasicBlock, IRInstruction, IROperand from vyper.venom.context import IRFunction -from vyper.venom.basicblock import IRBasicBlock -from vyper.venom.basicblock import IRInstruction -from vyper.venom.basicblock import IRLiteral -from vyper.venom.basicblock import IRVariable -from vyper.venom.basicblock import IROperand -from vyper.venom.basicblock import BB_TERMINATORS -from vyper.utils import OrderedSet -from dataclasses import dataclass -from collections import deque + @dataclass class _Expression: - first_inst : IRInstruction + first_inst: IRInstruction opcode: str - operands : list[IROperand] + operands: list[IROperand] def __eq__(self, other): if not isinstance(other, _Expression): return False return self.first_inst == other.first_inst - + def __hash__(self) -> int: return hash((self.opcode, *self.operands)) @@ -37,7 +33,7 @@ def __repr__(self) -> str: res += "]" return res - def contains_expr(self, expr : "_Expression") -> bool: + def contains_expr(self, expr: "_Expression") -> bool: for op in self.operands: if op == expr: return True @@ -45,25 +41,21 @@ def contains_expr(self, expr : "_Expression") -> bool: return True return False + class _BBLattice: - data : dict[IRInstruction, OrderedSet[_Expression]] - out : OrderedSet[_Expression] + data: dict[IRInstruction, OrderedSet[_Expression]] + out: OrderedSet[_Expression] in_cache: OrderedSet[_Expression] | None - def __init__(self, bb : IRBasicBlock): + def __init__(self, bb: IRBasicBlock): self.data = dict() self.out = OrderedSet() self.in_cache = None for inst in bb.instructions: self.data[inst] = OrderedSet() -_UNINTERESTING_OPCODES = [ - "store", - "param", - "offset", - "phi", - "nop", -] + +_UNINTERESTING_OPCODES = ["store", "param", "offset", "phi", "nop"] _ALL = ("storage", "transient", "memory", "immutables", "balance", "returndata") @@ -106,18 +98,19 @@ def __init__(self, bb : IRBasicBlock): class _FunctionLattice: - data : dict[IRBasicBlock, _BBLattice] + data: dict[IRBasicBlock, _BBLattice] - def __init__(self, function : IRFunction): + def __init__(self, function: IRFunction): self.data = dict() for bb in function.get_basic_blocks(): self.data[bb] = _BBLattice(bb) + class AvailableExpressionAnalysis(IRAnalysis): - expressions : OrderedSet[_Expression] = OrderedSet() + expressions: OrderedSet[_Expression] = OrderedSet() inst_to_expr: dict[IRInstruction, _Expression] = dict() dfg: DFGAnalysis - lattice : _FunctionLattice + lattice: _FunctionLattice def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): super().__init__(analyses_cache, function) @@ -132,33 +125,33 @@ def analyze(self, *args, **kwargs): worklist = deque() worklist.append(self.function.entry) while len(worklist) > 0: - bb : IRBasicBlock = worklist.popleft() + bb: IRBasicBlock = worklist.popleft() changed = self._handle_bb(bb) if changed: for out in bb.cfg_out: if out not in worklist: worklist.append(out) - - def _handle_bb(self, bb : IRBasicBlock) -> bool: - available_expr : OrderedSet[_Expression] = OrderedSet() + + def _handle_bb(self, bb: IRBasicBlock) -> bool: + available_expr: OrderedSet[_Expression] = OrderedSet() if len(bb.cfg_in) > 0: - available_expr = OrderedSet.intersection(*(self.lattice.data[in_bb].out for in_bb in bb.cfg_in)) - + available_expr = OrderedSet.intersection( + *(self.lattice.data[in_bb].out for in_bb in bb.cfg_in) + ) + bb_lat = self.lattice.data[bb] if bb_lat.in_cache is not None and available_expr == bb_lat.in_cache: return False bb_lat.in_cache = available_expr change = False for inst in bb.instructions: - if (inst.opcode in _UNINTERESTING_OPCODES - or inst.opcode in BB_TERMINATORS): + if inst.opcode in _UNINTERESTING_OPCODES or inst.opcode in BB_TERMINATORS: continue if available_expr != bb_lat.data[inst]: bb_lat.data[inst] = available_expr.copy() change |= True - inst_expr = self.get_expression(inst, available_expr) write_effects = writes.get(inst_expr.opcode, ()) for expr in available_expr.copy(): @@ -168,16 +161,18 @@ def _handle_bb(self, bb : IRBasicBlock) -> bool: if any(eff in write_effects for eff in read_effects): available_expr.remove(expr) - if "call" not in inst.opcode and inst.opcode != "invoke": + if "call" not in inst.opcode and inst.opcode not in ["invoke", "log"]: available_expr.add(inst_expr) if available_expr != bb_lat.out: bb_lat.out = available_expr.copy() change |= True - + return change - def get_expression(self, inst: IRInstruction, available_exprs : OrderedSet[_Expression] | None = None) -> _Expression: + def get_expression( + self, inst: IRInstruction, available_exprs: OrderedSet[_Expression] | None = None + ) -> _Expression: if available_exprs is None: available_exprs = self.lattice.data[inst.parent].data[inst] operands: list[IROperand] = inst.operands.copy() @@ -185,9 +180,8 @@ def get_expression(self, inst: IRInstruction, available_exprs : OrderedSet[_Expr for e in available_exprs: if e.opcode == expr.opcode and e.operands == expr.operands: return e - + return expr - def get_available(self, inst : IRInstruction) -> OrderedSet[_Expression]: + def get_available(self, inst: IRInstruction) -> OrderedSet[_Expression]: return self.lattice.data[inst.parent].data[inst] - diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index a03d2fa42f..ffdc58512f 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -1,20 +1,21 @@ -from vyper.venom.passes.base_pass import IRPass +from vyper.utils import OrderedSet from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis -from vyper.venom.analysis.liveness import LivenessAnalysis from vyper.venom.analysis.dfg import DFGAnalysis -from vyper.venom.basicblock import IRInstruction -from vyper.venom.basicblock import IRVariable -from vyper.venom.basicblock import IRBasicBlock -from vyper.utils import OrderedSet +from vyper.venom.analysis.liveness import LivenessAnalysis +from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRVariable +from vyper.venom.passes.base_pass import IRPass + class CSE(IRPass): - available_expression_analysis : AvailableExpressionAnalysis + available_expression_analysis: AvailableExpressionAnalysis def run_pass(self, *args, **kwargs): - available_expression_analysis = self.analyses_cache.request_analysis(AvailableExpressionAnalysis) + available_expression_analysis = self.analyses_cache.request_analysis( + AvailableExpressionAnalysis + ) assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) self.available_expression_analysis = available_expression_analysis - + while True: replace_dict = self._find_replaceble() if len(replace_dict) == 0: @@ -26,23 +27,23 @@ def run_pass(self, *args, **kwargs): # return instruction and to which instruction it could # replaced by def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: - res : dict[IRInstruction, IRInstruction] = dict() + res: dict[IRInstruction, IRInstruction] = dict() for bb in self.function.get_basic_blocks(): for inst in bb.instructions: inst_expr = self.available_expression_analysis.get_expression(inst) - avail = self.available_expression_analysis.get_available(inst) + avail = self.available_expression_analysis.get_available(inst) if inst_expr in avail: res[inst] = inst_expr.first_inst return res - def _replace(self, replace_dict : dict[IRInstruction, IRInstruction]): - for (orig, to) in replace_dict.items(): + def _replace(self, replace_dict: dict[IRInstruction, IRInstruction]): + for orig, to in replace_dict.items(): while to in replace_dict.keys(): to = replace_dict[to] self._replace_inst(orig, to) - def _replace_inst(self, orig_inst : IRInstruction, to_inst : IRInstruction): + def _replace_inst(self, orig_inst: IRInstruction, to_inst: IRInstruction): visited: OrderedSet[IRBasicBlock] = OrderedSet() if orig_inst.output is not None: assert isinstance(orig_inst.output, IRVariable), f"not var {orig_inst}" @@ -50,7 +51,9 @@ def _replace_inst(self, orig_inst : IRInstruction, to_inst : IRInstruction): self._replace_inst_r(orig_inst.parent, orig_inst.output, to_inst.output, visited) orig_inst.parent.remove_instruction(orig_inst) - def _replace_inst_r(self, bb : IRBasicBlock, orig : IRVariable, to : IRVariable, visited : OrderedSet[IRBasicBlock]): + def _replace_inst_r( + self, bb: IRBasicBlock, orig: IRVariable, to: IRVariable, visited: OrderedSet[IRBasicBlock] + ): if bb in visited: return visited.add(bb) From de3c6020c72d9b2e0ab681dda61ed3f66fb7eed2 Mon Sep 17 00:00:00 2001 From: plodeada Date: Wed, 18 Sep 2024 13:57:39 +0200 Subject: [PATCH 16/59] handling the different size of the expressions --- vyper/venom/analysis/available_expression.py | 73 +++++++++++++++++-- .../common_subexpression_elimination.py | 7 +- 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index d0486f253f..21ca770f34 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -5,15 +5,24 @@ from vyper.venom.analysis.analysis import IRAnalysesCache, IRAnalysis from vyper.venom.analysis.cfg import CFGAnalysis from vyper.venom.analysis.dfg import DFGAnalysis -from vyper.venom.basicblock import BB_TERMINATORS, IRBasicBlock, IRInstruction, IROperand +from vyper.venom.basicblock import ( + BB_TERMINATORS, + IRBasicBlock, + IRInstruction, + IROperand, + IRVariable, +) from vyper.venom.context import IRFunction +_MAX_DEPTH = 5 +_MIN_DEPTH = 2 + @dataclass class _Expression: first_inst: IRInstruction opcode: str - operands: list[IROperand] + operands: list["IROperand | _Expression"] def __eq__(self, other): if not isinstance(other, _Expression): @@ -33,6 +42,23 @@ def __repr__(self) -> str: res += "]" return res + def same(self, other: "_Expression") -> bool: + if self.opcode != other.opcode: + return False + for self_op, other_op in zip(self.operands, other.operands): + if type(self_op) is not type(other_op): + return False + if isinstance(self_op, _Expression): + assert isinstance(other_op, _Expression) + if not self_op.same(other_op): + return False + else: + assert isinstance(self_op, IROperand) + assert isinstance(other_op, IROperand) + if self_op != other_op: + return False + return True + def contains_expr(self, expr: "_Expression") -> bool: for op in self.operands: if op == expr: @@ -41,6 +67,15 @@ def contains_expr(self, expr: "_Expression") -> bool: return True return False + def get_depth(self) -> int: + max_depth = 0 + for op in self.operands: + if isinstance(op, _Expression): + d = op.get_depth() + if d > max_depth: + max_depth = d + return max_depth + 1 + class _BBLattice: data: dict[IRInstruction, OrderedSet[_Expression]] @@ -155,13 +190,17 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: inst_expr = self.get_expression(inst, available_expr) write_effects = writes.get(inst_expr.opcode, ()) for expr in available_expr.copy(): - if expr.contains_expr(inst_expr): - available_expr.remove(expr) + # if expr.contains_expr(inst_expr): + # available_expr.remove(expr) read_effects = reads.get(expr.opcode, ()) if any(eff in write_effects for eff in read_effects): available_expr.remove(expr) - if "call" not in inst.opcode and inst.opcode not in ["invoke", "log"]: + if ( + "call" not in inst.opcode + and inst.opcode not in ["invoke", "log"] + and inst_expr.get_depth() in range(_MIN_DEPTH, _MAX_DEPTH + 1) + ): available_expr.add(inst_expr) if available_expr != bb_lat.out: @@ -170,15 +209,33 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: return change + def _get_operand( + self, op: IROperand, available_exprs: OrderedSet[_Expression], depth: int + ) -> IROperand | _Expression: + if depth > 0 and isinstance(op, IRVariable): + inst = self.dfg.get_producing_instruction(op) + assert inst is not None + return self.get_expression(inst, available_exprs, depth - 1) + return op + + def _get_operands( + self, inst: IRInstruction, available_exprs: OrderedSet[_Expression], depth: int = _MAX_DEPTH + ) -> list[IROperand | _Expression]: + return [self._get_operand(op, available_exprs, depth) for op in inst.operands] + def get_expression( - self, inst: IRInstruction, available_exprs: OrderedSet[_Expression] | None = None + self, + inst: IRInstruction, + available_exprs: OrderedSet[_Expression] | None = None, + depth: int = _MAX_DEPTH, ) -> _Expression: if available_exprs is None: available_exprs = self.lattice.data[inst.parent].data[inst] - operands: list[IROperand] = inst.operands.copy() + operands: list[IROperand | _Expression] = self._get_operands(inst, available_exprs, depth) expr = _Expression(inst, inst.opcode, operands) for e in available_exprs: - if e.opcode == expr.opcode and e.operands == expr.operands: + # if e.opcode == expr.opcode and e.operands == expr.operands: + if expr.same(e): return e return expr diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index ffdc58512f..8d8bf348d6 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -1,5 +1,8 @@ from vyper.utils import OrderedSet -from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis +from vyper.venom.analysis.available_expression import ( + _UNINTERESTING_OPCODES, + AvailableExpressionAnalysis, +) from vyper.venom.analysis.dfg import DFGAnalysis from vyper.venom.analysis.liveness import LivenessAnalysis from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRVariable @@ -30,6 +33,8 @@ def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: res: dict[IRInstruction, IRInstruction] = dict() for bb in self.function.get_basic_blocks(): for inst in bb.instructions: + if inst in _UNINTERESTING_OPCODES: + continue inst_expr = self.available_expression_analysis.get_expression(inst) avail = self.available_expression_analysis.get_available(inst) if inst_expr in avail: From b286eeeff6d40d108998d05473c72781419626c1 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 23 Sep 2024 22:54:07 +0200 Subject: [PATCH 17/59] fixes in handling effects + bigger expression --- .../test_common_subexpression_elimination.py | 23 ++++++ vyper/venom/__init__.py | 1 - vyper/venom/analysis/available_expression.py | 81 +++++++++++-------- .../common_subexpression_elimination.py | 7 ++ 4 files changed, 79 insertions(+), 33 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index d72218ae7f..226730d271 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -18,6 +18,29 @@ def test_common_subexpression_elimination(): ac = IRAnalysesCache(fn) CSE(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() + print(fn) assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 1, "wrong number of adds" assert sum(1 for inst in bb.instructions if inst.opcode == "mul") == 1, "wrong number of muls" + +def test_common_subexpression_elimination_effects(): + ctx = IRContext() + fn = ctx.create_function("test") + bb = fn.get_basic_block() + mload_1 = bb.append_instruction("mload", 0) + op = bb.append_instruction("store", 10) + bb.append_instruction("mstore", op, 0) + mload_2 = bb.append_instruction("mload", 0) + bb.append_instruction("add", mload_1, 10) + bb.append_instruction("add", mload_2, 10) + bb.append_instruction("stop") + + ac = IRAnalysesCache(fn) + CSE(ac, fn).run_pass() + ExtractLiteralsPass(ac, fn).run_pass() + print(fn) + + assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" + #assert False + #assert sum(1 for inst in bb.instructions if inst.opcode == "mul") == 1, "wrong number of muls" + diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 0bbae0a402..166508a989 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -55,7 +55,6 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: SimplifyCFGPass(ac, fn).run_pass() AlgebraicOptimizationPass(ac, fn).run_pass() BranchOptimizationPass(ac, fn).run_pass() - RemoveUnusedVariablesPass(ac, fn).run_pass() CSE(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 21ca770f34..39eeb50d5b 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -76,6 +76,23 @@ def get_depth(self) -> int: max_depth = d return max_depth + 1 + def get_effects(self) -> list[str]: + tmp_effects: set[str] = set(reads.get(self.opcode, ())) + tmp_effects: set[str] = tmp_effects.union(writes.get(self.opcode, ())) + for op in self.operands: + if isinstance(op, IRVariable): + return list(_ALL) + if isinstance(op, _Expression): + tmp_effects = tmp_effects.union(op.get_effects()) + return list(tmp_effects) + + def get_reads(self) -> list[str]: + tmp_reads: set[str] = set(reads.get(self.opcode, ())) + for op in self.operands: + if isinstance(op, _Expression): + tmp_reads = tmp_reads.union(op.get_reads()) + return list(tmp_reads) + class _BBLattice: data: dict[IRInstruction, OrderedSet[_Expression]] @@ -92,43 +109,44 @@ def __init__(self, bb: IRBasicBlock): _UNINTERESTING_OPCODES = ["store", "param", "offset", "phi", "nop"] -_ALL = ("storage", "transient", "memory", "immutables", "balance", "returndata") +_ALL = ("storage", "transient", "memory", "immutables", "balance", "returndata", "log") writes = { - "sstore": ("storage"), - "tstore": ("transient"), - "mstore": ("memory"), - "istore": ("immutables"), + "sstore": ("storage",), + "tstore": ("transient",), + "mstore": ("memory",), + "istore": ("immutables",), "call": _ALL, "delegatecall": _ALL, - "staticcall": ("memory"), + "staticcall": ("memory", "returndata"), "create": _ALL, "create2": _ALL, "invoke": _ALL, # could be smarter, look up the effects of the invoked function - "dloadbytes": ("memory"), - "returndatacopy": ("memory"), - "calldatacopy": ("memory"), - "codecopy": ("memory"), - "extcodecopy": ("memory"), - "mcopy": ("memory"), + "dloadbytes": ("memory",), + "returndatacopy": ("memory",), + "calldatacopy": ("memory",), + "codecopy": ("memory",), + "extcodecopy": ("memory",), + "mcopy": ("memory",), + "log": ("log",) } reads = { - "sload": ("storage"), - "tload": ("transient"), - "iload": ("immutables"), - "mload": ("memory"), - "mcopy": ("memory"), + "sload": ("storage",), + "tload": ("transient",), + "iload": ("immutables",), + "mload": ("memory",), + "mcopy": ("memory",), "call": _ALL, "delegatecall": _ALL, "staticcall": _ALL, - "returndatasize": ("returndata"), - "returndatacopy": ("returndata"), - "balance": ("balance"), - "selfbalance": ("balance"), - "log": ("memory"), - "revert": ("memory"), - "return": ("memory"), - "sha3": ("memory"), + "returndatasize": ("returndata",), + "returndatacopy": ("returndata",), + "balance": ("balance",), + "selfbalance": ("balance",), + "log": ("memory",), + "revert": ("memory",), + "return": ("memory",), + "sha3": ("memory",), } @@ -190,16 +208,13 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: inst_expr = self.get_expression(inst, available_expr) write_effects = writes.get(inst_expr.opcode, ()) for expr in available_expr.copy(): - # if expr.contains_expr(inst_expr): - # available_expr.remove(expr) - read_effects = reads.get(expr.opcode, ()) + read_effects = expr.get_effects() if any(eff in write_effects for eff in read_effects): available_expr.remove(expr) if ( - "call" not in inst.opcode - and inst.opcode not in ["invoke", "log"] - and inst_expr.get_depth() in range(_MIN_DEPTH, _MAX_DEPTH + 1) + inst_expr.get_depth() in range(_MIN_DEPTH, _MAX_DEPTH + 1) + and not any(eff in write_effects for eff in inst_expr.get_effects()) ): available_expr.add(inst_expr) @@ -215,7 +230,9 @@ def _get_operand( if depth > 0 and isinstance(op, IRVariable): inst = self.dfg.get_producing_instruction(op) assert inst is not None - return self.get_expression(inst, available_exprs, depth - 1) + #if inst in available_exprs or inst.opcode in _UNINTERESTING_OPCODES: + if not inst.is_volatile: + return self.get_expression(inst, available_exprs, depth - 1) return op def _get_operands( diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 8d8bf348d6..92b771fe94 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -26,6 +26,13 @@ def run_pass(self, *args, **kwargs): self._replace(replace_dict) self.analyses_cache.invalidate_analysis(DFGAnalysis) self.analyses_cache.invalidate_analysis(LivenessAnalysis) + #self.analyses_cache.invalidate_analysis(AvailableExpressionAnalysis) + available_expression_analysis = self.analyses_cache.request_analysis( + AvailableExpressionAnalysis + ) + assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) + self.available_expression_analysis = available_expression_analysis + # return instruction and to which instruction it could # replaced by From 14b8963f7c160b41054b6c2056ccbad50d567241 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 24 Sep 2024 14:39:12 +0200 Subject: [PATCH 18/59] fixes in handling effects --- .../test_common_subexpression_elimination.py | 47 +++++++++++++++++-- vyper/venom/__init__.py | 3 ++ vyper/venom/analysis/available_expression.py | 41 +++++++++------- .../common_subexpression_elimination.py | 3 +- 4 files changed, 72 insertions(+), 22 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 226730d271..9366738b1c 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -1,6 +1,8 @@ from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis from vyper.venom.context import IRContext from vyper.venom.passes.common_subexpression_elimination import CSE +from vyper.venom.passes.dft import DFTPass from vyper.venom.passes.extract_literals import ExtractLiteralsPass @@ -23,7 +25,8 @@ def test_common_subexpression_elimination(): assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 1, "wrong number of adds" assert sum(1 for inst in bb.instructions if inst.opcode == "mul") == 1, "wrong number of muls" -def test_common_subexpression_elimination_effects(): + +def test_common_subexpression_elimination_effects_1(): ctx = IRContext() fn = ctx.create_function("test") bb = fn.get_basic_block() @@ -36,11 +39,49 @@ def test_common_subexpression_elimination_effects(): bb.append_instruction("stop") ac = IRAnalysesCache(fn) + + print(fn) + + avail: AvailableExpressionAnalysis = ac.force_analysis(AvailableExpressionAnalysis) + + for inst in bb.instructions: + print(inst, avail.get_available(inst)) + + DFTPass(ac, fn).run_pass() CSE(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() print(fn) + avail: AvailableExpressionAnalysis = ac.force_analysis(AvailableExpressionAnalysis) + + for inst in bb.instructions: + print(inst, avail.get_available(inst)) + assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" - #assert False - #assert sum(1 for inst in bb.instructions if inst.opcode == "mul") == 1, "wrong number of muls" + +def test_common_subexpression_elimination_effects_2(): + ctx = IRContext() + fn = ctx.create_function("test") + bb = fn.get_basic_block() + mload_1 = bb.append_instruction("mload", 0) + bb.append_instruction("add", mload_1, 10) + op = bb.append_instruction("store", 10) + bb.append_instruction("mstore", op, 0) + mload_2 = bb.append_instruction("mload", 0) + bb.append_instruction("add", mload_1, 10) + bb.append_instruction("add", mload_2, 10) + bb.append_instruction("stop") + + ac = IRAnalysesCache(fn) + DFTPass(ac, fn).run_pass() + CSE(ac, fn).run_pass() + ExtractLiteralsPass(ac, fn).run_pass() + print(fn) + + avail: AvailableExpressionAnalysis = ac.force_analysis(AvailableExpressionAnalysis) + + for inst in bb.instructions: + print(inst, avail.get_available(inst)) + + assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 166508a989..1e58eaa567 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -55,6 +55,9 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: SimplifyCFGPass(ac, fn).run_pass() AlgebraicOptimizationPass(ac, fn).run_pass() BranchOptimizationPass(ac, fn).run_pass() + + RemoveUnusedVariablesPass(ac, fn).run_pass() + DFTPass(ac, fn).run_pass() CSE(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 39eeb50d5b..a4d7794aac 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -36,7 +36,11 @@ def __repr__(self) -> str: if self.opcode == "store": assert len(self.operands) == 1, "wrong store" return repr(self.operands[0]) - res = self.opcode + " [ " + res = ( + self.opcode + f"({self.first_inst.fence_id})" + if self.first_inst.is_volatile + else "" + " [ " + ) for op in self.operands: res += repr(op) + " " res += "]" @@ -45,6 +49,10 @@ def __repr__(self) -> str: def same(self, other: "_Expression") -> bool: if self.opcode != other.opcode: return False + if ( + self.first_inst.is_volatile or "returndata" in self.opcode + ) and self.first_inst.fence_id != other.first_inst.fence_id: + return False for self_op, other_op in zip(self.operands, other.operands): if type(self_op) is not type(other_op): return False @@ -78,19 +86,19 @@ def get_depth(self) -> int: def get_effects(self) -> list[str]: tmp_effects: set[str] = set(reads.get(self.opcode, ())) - tmp_effects: set[str] = tmp_effects.union(writes.get(self.opcode, ())) - for op in self.operands: - if isinstance(op, IRVariable): - return list(_ALL) - if isinstance(op, _Expression): - tmp_effects = tmp_effects.union(op.get_effects()) + tmp_effects = tmp_effects.union(writes.get(self.opcode, ())) + # for op in self.operands: + # if isinstance(op, IRVariable): + # return list(_ALL) + # if isinstance(op, _Expression): + # tmp_effects = tmp_effects.union(op.get_effects()) return list(tmp_effects) def get_reads(self) -> list[str]: tmp_reads: set[str] = set(reads.get(self.opcode, ())) - for op in self.operands: - if isinstance(op, _Expression): - tmp_reads = tmp_reads.union(op.get_reads()) + # for op in self.operands: + # if isinstance(op, _Expression): + # tmp_reads = tmp_reads.union(op.get_reads()) return list(tmp_reads) @@ -128,7 +136,7 @@ def __init__(self, bb: IRBasicBlock): "codecopy": ("memory",), "extcodecopy": ("memory",), "mcopy": ("memory",), - "log": ("log",) + "log": ("log",), } reads = { "sload": ("storage",), @@ -212,9 +220,8 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: if any(eff in write_effects for eff in read_effects): available_expr.remove(expr) - if ( - inst_expr.get_depth() in range(_MIN_DEPTH, _MAX_DEPTH + 1) - and not any(eff in write_effects for eff in inst_expr.get_effects()) + if inst_expr.get_depth() in range(_MIN_DEPTH, _MAX_DEPTH + 1) and not any( + eff in write_effects for eff in inst_expr.get_effects() ): available_expr.add(inst_expr) @@ -230,9 +237,9 @@ def _get_operand( if depth > 0 and isinstance(op, IRVariable): inst = self.dfg.get_producing_instruction(op) assert inst is not None - #if inst in available_exprs or inst.opcode in _UNINTERESTING_OPCODES: - if not inst.is_volatile: - return self.get_expression(inst, available_exprs, depth - 1) + # if inst in available_exprs or inst.opcode in _UNINTERESTING_OPCODES: + # if not inst.is_volatile: + return self.get_expression(inst, available_exprs, depth - 1) return op def _get_operands( diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 92b771fe94..f1677e7123 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -26,14 +26,12 @@ def run_pass(self, *args, **kwargs): self._replace(replace_dict) self.analyses_cache.invalidate_analysis(DFGAnalysis) self.analyses_cache.invalidate_analysis(LivenessAnalysis) - #self.analyses_cache.invalidate_analysis(AvailableExpressionAnalysis) available_expression_analysis = self.analyses_cache.request_analysis( AvailableExpressionAnalysis ) assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) self.available_expression_analysis = available_expression_analysis - # return instruction and to which instruction it could # replaced by def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: @@ -53,6 +51,7 @@ def _replace(self, replace_dict: dict[IRInstruction, IRInstruction]): for orig, to in replace_dict.items(): while to in replace_dict.keys(): to = replace_dict[to] + # print(orig.output, to.output) self._replace_inst(orig, to) def _replace_inst(self, orig_inst: IRInstruction, to_inst: IRInstruction): From e957b045ba6f7c364d4abdc038801105c76b9bc5 Mon Sep 17 00:00:00 2001 From: plodeada Date: Wed, 25 Sep 2024 16:09:41 +0200 Subject: [PATCH 19/59] fixes (to better version) and style changes --- .../test_common_subexpression_elimination.py | 4 +++ vyper/venom/__init__.py | 3 -- vyper/venom/analysis/available_expression.py | 32 +++++++------------ .../common_subexpression_elimination.py | 1 - 4 files changed, 15 insertions(+), 25 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 9366738b1c..0b212ed8d1 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -1,3 +1,5 @@ +import pytest + from vyper.venom.analysis.analysis import IRAnalysesCache from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis from vyper.venom.context import IRContext @@ -60,6 +62,8 @@ def test_common_subexpression_elimination_effects_1(): assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" +# This is a limitation of current implementation +@pytest.mark.xfail def test_common_subexpression_elimination_effects_2(): ctx = IRContext() fn = ctx.create_function("test") diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 1e58eaa567..166508a989 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -55,9 +55,6 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: SimplifyCFGPass(ac, fn).run_pass() AlgebraicOptimizationPass(ac, fn).run_pass() BranchOptimizationPass(ac, fn).run_pass() - - RemoveUnusedVariablesPass(ac, fn).run_pass() - DFTPass(ac, fn).run_pass() CSE(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index a4d7794aac..3a1dac20b5 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -36,11 +36,7 @@ def __repr__(self) -> str: if self.opcode == "store": assert len(self.operands) == 1, "wrong store" return repr(self.operands[0]) - res = ( - self.opcode + f"({self.first_inst.fence_id})" - if self.first_inst.is_volatile - else "" + " [ " - ) + res = self.opcode + " [ " for op in self.operands: res += repr(op) + " " res += "]" @@ -49,10 +45,6 @@ def __repr__(self) -> str: def same(self, other: "_Expression") -> bool: if self.opcode != other.opcode: return False - if ( - self.first_inst.is_volatile or "returndata" in self.opcode - ) and self.first_inst.fence_id != other.first_inst.fence_id: - return False for self_op, other_op in zip(self.operands, other.operands): if type(self_op) is not type(other_op): return False @@ -87,18 +79,18 @@ def get_depth(self) -> int: def get_effects(self) -> list[str]: tmp_effects: set[str] = set(reads.get(self.opcode, ())) tmp_effects = tmp_effects.union(writes.get(self.opcode, ())) - # for op in self.operands: - # if isinstance(op, IRVariable): - # return list(_ALL) - # if isinstance(op, _Expression): - # tmp_effects = tmp_effects.union(op.get_effects()) + for op in self.operands: + if isinstance(op, IRVariable): + return list(_ALL) + if isinstance(op, _Expression): + tmp_effects = tmp_effects.union(op.get_effects()) return list(tmp_effects) def get_reads(self) -> list[str]: tmp_reads: set[str] = set(reads.get(self.opcode, ())) - # for op in self.operands: - # if isinstance(op, _Expression): - # tmp_reads = tmp_reads.union(op.get_reads()) + for op in self.operands: + if isinstance(op, _Expression): + tmp_reads = tmp_reads.union(op.get_reads()) return list(tmp_reads) @@ -237,9 +229,8 @@ def _get_operand( if depth > 0 and isinstance(op, IRVariable): inst = self.dfg.get_producing_instruction(op) assert inst is not None - # if inst in available_exprs or inst.opcode in _UNINTERESTING_OPCODES: - # if not inst.is_volatile: - return self.get_expression(inst, available_exprs, depth - 1) + if not inst.is_volatile: + return self.get_expression(inst, available_exprs, depth - 1) return op def _get_operands( @@ -258,7 +249,6 @@ def get_expression( operands: list[IROperand | _Expression] = self._get_operands(inst, available_exprs, depth) expr = _Expression(inst, inst.opcode, operands) for e in available_exprs: - # if e.opcode == expr.opcode and e.operands == expr.operands: if expr.same(e): return e diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index f1677e7123..2339077be9 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -51,7 +51,6 @@ def _replace(self, replace_dict: dict[IRInstruction, IRInstruction]): for orig, to in replace_dict.items(): while to in replace_dict.keys(): to = replace_dict[to] - # print(orig.output, to.output) self._replace_inst(orig, to) def _replace_inst(self, orig_inst: IRInstruction, to_inst: IRInstruction): From 63f27fec73adce29bfb8aec43e008738c0b4cff7 Mon Sep 17 00:00:00 2001 From: plodeada Date: Thu, 26 Sep 2024 11:40:17 +0200 Subject: [PATCH 20/59] perf fixes + better order --- .../test_common_subexpression_elimination.py | 51 ++++++++++--------- vyper/venom/__init__.py | 2 +- vyper/venom/analysis/available_expression.py | 4 +- .../common_subexpression_elimination.py | 2 +- 4 files changed, 30 insertions(+), 29 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 0b212ed8d1..312730ce86 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -1,10 +1,8 @@ import pytest from vyper.venom.analysis.analysis import IRAnalysesCache -from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis from vyper.venom.context import IRContext from vyper.venom.passes.common_subexpression_elimination import CSE -from vyper.venom.passes.dft import DFTPass from vyper.venom.passes.extract_literals import ExtractLiteralsPass @@ -22,7 +20,6 @@ def test_common_subexpression_elimination(): ac = IRAnalysesCache(fn) CSE(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() - print(fn) assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 1, "wrong number of adds" assert sum(1 for inst in bb.instructions if inst.opcode == "mul") == 1, "wrong number of muls" @@ -42,22 +39,8 @@ def test_common_subexpression_elimination_effects_1(): ac = IRAnalysesCache(fn) - print(fn) - - avail: AvailableExpressionAnalysis = ac.force_analysis(AvailableExpressionAnalysis) - - for inst in bb.instructions: - print(inst, avail.get_available(inst)) - - DFTPass(ac, fn).run_pass() - CSE(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() - print(fn) - - avail: AvailableExpressionAnalysis = ac.force_analysis(AvailableExpressionAnalysis) - - for inst in bb.instructions: - print(inst, avail.get_available(inst)) + CSE(ac, fn).run_pass() assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" @@ -78,14 +61,32 @@ def test_common_subexpression_elimination_effects_2(): bb.append_instruction("stop") ac = IRAnalysesCache(fn) - DFTPass(ac, fn).run_pass() - CSE(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() - print(fn) + CSE(ac, fn).run_pass() - avail: AvailableExpressionAnalysis = ac.force_analysis(AvailableExpressionAnalysis) + assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" - for inst in bb.instructions: - print(inst, avail.get_available(inst)) - assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" +def test_common_subexpression_elimination_effect_mstore(): + ctx = IRContext() + fn = ctx.create_function("test") + bb = fn.get_basic_block() + op = bb.append_instruction("store", 10) + bb.append_instruction("mstore", op, 0) + mload_1 = bb.append_instruction("mload", 0) + op = bb.append_instruction("store", 10) + bb.append_instruction("mstore", op, 0) + mload_2 = bb.append_instruction("mload", 0) + bb.append_instruction("add", mload_1, mload_2) + bb.append_instruction("stop") + + ac = IRAnalysesCache(fn) + ExtractLiteralsPass(ac, fn).run_pass() + CSE(ac, fn).run_pass() + + assert ( + sum(1 for inst in bb.instructions if inst.opcode == "mstore") == 1 + ), "wrong number of mstores" + assert ( + sum(1 for inst in bb.instructions if inst.opcode == "mload") == 1 + ), "wrong number of mloads" diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 166508a989..db8dc71fb0 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -55,8 +55,8 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: SimplifyCFGPass(ac, fn).run_pass() AlgebraicOptimizationPass(ac, fn).run_pass() BranchOptimizationPass(ac, fn).run_pass() - CSE(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() + CSE(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() DFTPass(ac, fn).run_pass() diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 3a1dac20b5..b7ada63618 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -143,7 +143,7 @@ def __init__(self, bb: IRBasicBlock): "returndatacopy": ("returndata",), "balance": ("balance",), "selfbalance": ("balance",), - "log": ("memory",), + "log": ("memory", "log"), # I think here about log as a append to a log "revert": ("memory",), "return": ("memory",), "sha3": ("memory",), @@ -213,7 +213,7 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: available_expr.remove(expr) if inst_expr.get_depth() in range(_MIN_DEPTH, _MAX_DEPTH + 1) and not any( - eff in write_effects for eff in inst_expr.get_effects() + eff in write_effects for eff in inst_expr.get_reads() ): available_expr.add(inst_expr) diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 2339077be9..f0bf5afb01 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -26,7 +26,7 @@ def run_pass(self, *args, **kwargs): self._replace(replace_dict) self.analyses_cache.invalidate_analysis(DFGAnalysis) self.analyses_cache.invalidate_analysis(LivenessAnalysis) - available_expression_analysis = self.analyses_cache.request_analysis( + available_expression_analysis = self.analyses_cache.force_analysis( AvailableExpressionAnalysis ) assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) From 9ce9dd8773b47b02b0098e12565380d531e0b29a Mon Sep 17 00:00:00 2001 From: plodeada Date: Thu, 26 Sep 2024 15:17:37 +0200 Subject: [PATCH 21/59] quick fix just for dft but since it is being rework it is just hot fix --- vyper/venom/passes/dft.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index f45a60079c..2c8e53f2a6 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -46,7 +46,7 @@ def _process_instruction_r(self, bb: IRBasicBlock, inst: IRInstruction, offset: if target.parent != inst.parent or target.fence_id != inst.fence_id: # don't reorder across basic block or fence boundaries continue - self._process_instruction_r(bb, target, offset) + self._process_instruction_r(bb, target, 0) self.inst_order[inst] = self.inst_order_num + offset From 775f0a9de3d824902c501db4d9336b49c3218f43 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 30 Sep 2024 15:17:14 +0200 Subject: [PATCH 22/59] created possibility to change size of the expression at pass level --- vyper/venom/__init__.py | 2 +- vyper/venom/analysis/available_expression.py | 20 +++++++++++++------ .../common_subexpression_elimination.py | 12 ++++++++--- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index db8dc71fb0..f4efa31245 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -56,7 +56,7 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: AlgebraicOptimizationPass(ac, fn).run_pass() BranchOptimizationPass(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() - CSE(ac, fn).run_pass() + CSE(ac, fn).run_pass(2, 10) RemoveUnusedVariablesPass(ac, fn).run_pass() DFTPass(ac, fn).run_pass() diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index b7ada63618..12db7a23ae 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -17,7 +17,6 @@ _MAX_DEPTH = 5 _MIN_DEPTH = 2 - @dataclass class _Expression: first_inst: IRInstruction @@ -164,17 +163,24 @@ class AvailableExpressionAnalysis(IRAnalysis): inst_to_expr: dict[IRInstruction, _Expression] = dict() dfg: DFGAnalysis lattice: _FunctionLattice + min_depth: int + max_depth: int - def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): + def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction, min_depth : int = _MIN_DEPTH, max_depth : int = _MAX_DEPTH): super().__init__(analyses_cache, function) self.analyses_cache.request_analysis(CFGAnalysis) dfg = self.analyses_cache.request_analysis(DFGAnalysis) assert isinstance(dfg, DFGAnalysis) self.dfg = dfg + self.min_depth = min_depth + self.max_depth = max_depth + self.lattice = _FunctionLattice(function) - def analyze(self, *args, **kwargs): + def analyze(self, min_depth : int = _MIN_DEPTH, max_depth : int = _MAX_DEPTH): + self.min_depth = min_depth + self.max_depth = max_depth worklist = deque() worklist.append(self.function.entry) while len(worklist) > 0: @@ -212,7 +218,7 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: if any(eff in write_effects for eff in read_effects): available_expr.remove(expr) - if inst_expr.get_depth() in range(_MIN_DEPTH, _MAX_DEPTH + 1) and not any( + if inst_expr.get_depth() in range(self.min_depth, self.max_depth + 1) and not any( eff in write_effects for eff in inst_expr.get_reads() ): available_expr.add(inst_expr) @@ -234,7 +240,7 @@ def _get_operand( return op def _get_operands( - self, inst: IRInstruction, available_exprs: OrderedSet[_Expression], depth: int = _MAX_DEPTH + self, inst: IRInstruction, available_exprs: OrderedSet[_Expression], depth: int ) -> list[IROperand | _Expression]: return [self._get_operand(op, available_exprs, depth) for op in inst.operands] @@ -242,10 +248,12 @@ def get_expression( self, inst: IRInstruction, available_exprs: OrderedSet[_Expression] | None = None, - depth: int = _MAX_DEPTH, + depth: int | None = None, ) -> _Expression: if available_exprs is None: available_exprs = self.lattice.data[inst.parent].data[inst] + if depth is None: + depth = self.max_depth operands: list[IROperand | _Expression] = self._get_operands(inst, available_exprs, depth) expr = _Expression(inst, inst.opcode, operands) for e in available_exprs: diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index f0bf5afb01..dd15debfe0 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -8,13 +8,17 @@ from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRVariable from vyper.venom.passes.base_pass import IRPass +_MAX_DEPTH = 5 +_MIN_DEPTH = 2 class CSE(IRPass): available_expression_analysis: AvailableExpressionAnalysis - def run_pass(self, *args, **kwargs): + def run_pass(self, min_depth : int = _MIN_DEPTH, max_depth : int = _MAX_DEPTH): available_expression_analysis = self.analyses_cache.request_analysis( - AvailableExpressionAnalysis + AvailableExpressionAnalysis, + min_depth, + max_depth, ) assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) self.available_expression_analysis = available_expression_analysis @@ -27,7 +31,9 @@ def run_pass(self, *args, **kwargs): self.analyses_cache.invalidate_analysis(DFGAnalysis) self.analyses_cache.invalidate_analysis(LivenessAnalysis) available_expression_analysis = self.analyses_cache.force_analysis( - AvailableExpressionAnalysis + AvailableExpressionAnalysis, + min_depth, + max_depth ) assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) self.available_expression_analysis = available_expression_analysis From ea944e6d32814bd22f86d8d7672c7262fbe5d3d9 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 30 Sep 2024 15:24:57 +0200 Subject: [PATCH 23/59] created possibility to change size of the expression at pass level (lint) --- vyper/venom/analysis/available_expression.py | 13 ++++++++++--- .../passes/common_subexpression_elimination.py | 11 ++++------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 12db7a23ae..5e767f6ad9 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -17,6 +17,7 @@ _MAX_DEPTH = 5 _MIN_DEPTH = 2 + @dataclass class _Expression: first_inst: IRInstruction @@ -166,7 +167,13 @@ class AvailableExpressionAnalysis(IRAnalysis): min_depth: int max_depth: int - def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction, min_depth : int = _MIN_DEPTH, max_depth : int = _MAX_DEPTH): + def __init__( + self, + analyses_cache: IRAnalysesCache, + function: IRFunction, + min_depth: int = _MIN_DEPTH, + max_depth: int = _MAX_DEPTH, + ): super().__init__(analyses_cache, function) self.analyses_cache.request_analysis(CFGAnalysis) dfg = self.analyses_cache.request_analysis(DFGAnalysis) @@ -178,10 +185,10 @@ def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction, min_de self.lattice = _FunctionLattice(function) - def analyze(self, min_depth : int = _MIN_DEPTH, max_depth : int = _MAX_DEPTH): + def analyze(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): self.min_depth = min_depth self.max_depth = max_depth - worklist = deque() + worklist: deque = deque() worklist.append(self.function.entry) while len(worklist) > 0: bb: IRBasicBlock = worklist.popleft() diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index dd15debfe0..84a7193602 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -11,14 +11,13 @@ _MAX_DEPTH = 5 _MIN_DEPTH = 2 + class CSE(IRPass): available_expression_analysis: AvailableExpressionAnalysis - def run_pass(self, min_depth : int = _MIN_DEPTH, max_depth : int = _MAX_DEPTH): + def run_pass(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): available_expression_analysis = self.analyses_cache.request_analysis( - AvailableExpressionAnalysis, - min_depth, - max_depth, + AvailableExpressionAnalysis, min_depth, max_depth ) assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) self.available_expression_analysis = available_expression_analysis @@ -31,9 +30,7 @@ def run_pass(self, min_depth : int = _MIN_DEPTH, max_depth : int = _MAX_DEPTH): self.analyses_cache.invalidate_analysis(DFGAnalysis) self.analyses_cache.invalidate_analysis(LivenessAnalysis) available_expression_analysis = self.analyses_cache.force_analysis( - AvailableExpressionAnalysis, - min_depth, - max_depth + AvailableExpressionAnalysis, min_depth, max_depth ) assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) self.available_expression_analysis = available_expression_analysis From 018b6a96b160fa579ec91d28c6d1dfd77fb10f4b Mon Sep 17 00:00:00 2001 From: Hodan Date: Sat, 5 Oct 2024 20:06:07 +0200 Subject: [PATCH 24/59] fixes after merge --- .../test_common_subexpression_elimination.py | 18 +++++++++++------- vyper/venom/__init__.py | 1 - 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 312730ce86..60cbce444c 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -3,7 +3,6 @@ from vyper.venom.analysis.analysis import IRAnalysesCache from vyper.venom.context import IRContext from vyper.venom.passes.common_subexpression_elimination import CSE -from vyper.venom.passes.extract_literals import ExtractLiteralsPass def test_common_subexpression_elimination(): @@ -18,8 +17,13 @@ def test_common_subexpression_elimination(): bb.append_instruction("stop") ac = IRAnalysesCache(fn) - CSE(ac, fn).run_pass() - ExtractLiteralsPass(ac, fn).run_pass() + from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis + avail: AvailableExpressionAnalysis = ac.request_analysis(AvailableExpressionAnalysis) + print(fn) + for inst in bb.instructions: + print(avail.get_available(inst)) + + CSE(ac, fn).run_pass(1, 5) assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 1, "wrong number of adds" assert sum(1 for inst in bb.instructions if inst.opcode == "mul") == 1, "wrong number of muls" @@ -39,7 +43,7 @@ def test_common_subexpression_elimination_effects_1(): ac = IRAnalysesCache(fn) - ExtractLiteralsPass(ac, fn).run_pass() + #ExtractLiteralsPass(ac, fn).run_pass() CSE(ac, fn).run_pass() assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" @@ -61,7 +65,7 @@ def test_common_subexpression_elimination_effects_2(): bb.append_instruction("stop") ac = IRAnalysesCache(fn) - ExtractLiteralsPass(ac, fn).run_pass() + #ExtractLiteralsPass(ac, fn).run_pass() CSE(ac, fn).run_pass() assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" @@ -81,8 +85,8 @@ def test_common_subexpression_elimination_effect_mstore(): bb.append_instruction("stop") ac = IRAnalysesCache(fn) - ExtractLiteralsPass(ac, fn).run_pass() - CSE(ac, fn).run_pass() + #ExtractLiteralsPass(ac, fn).run_pass() + CSE(ac, fn).run_pass(1, 5) assert ( sum(1 for inst in bb.instructions if inst.opcode == "mstore") == 1 diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index e079f1d058..7970c4fc62 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -55,7 +55,6 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: SimplifyCFGPass(ac, fn).run_pass() AlgebraicOptimizationPass(ac, fn).run_pass() BranchOptimizationPass(ac, fn).run_pass() - ExtractLiteralsPass(ac, fn).run_pass() CSE(ac, fn).run_pass(2, 10) RemoveUnusedVariablesPass(ac, fn).run_pass() From 94a9a6ec19d3ff737c59d17f1148f31d2a9458a5 Mon Sep 17 00:00:00 2001 From: Hodan Date: Sun, 6 Oct 2024 09:47:40 +0200 Subject: [PATCH 25/59] better results this order --- .../venom/test_common_subexpression_elimination.py | 8 -------- vyper/venom/__init__.py | 3 ++- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 60cbce444c..5ce8fc367f 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -17,11 +17,6 @@ def test_common_subexpression_elimination(): bb.append_instruction("stop") ac = IRAnalysesCache(fn) - from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis - avail: AvailableExpressionAnalysis = ac.request_analysis(AvailableExpressionAnalysis) - print(fn) - for inst in bb.instructions: - print(avail.get_available(inst)) CSE(ac, fn).run_pass(1, 5) @@ -43,7 +38,6 @@ def test_common_subexpression_elimination_effects_1(): ac = IRAnalysesCache(fn) - #ExtractLiteralsPass(ac, fn).run_pass() CSE(ac, fn).run_pass() assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" @@ -65,7 +59,6 @@ def test_common_subexpression_elimination_effects_2(): bb.append_instruction("stop") ac = IRAnalysesCache(fn) - #ExtractLiteralsPass(ac, fn).run_pass() CSE(ac, fn).run_pass() assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" @@ -85,7 +78,6 @@ def test_common_subexpression_elimination_effect_mstore(): bb.append_instruction("stop") ac = IRAnalysesCache(fn) - #ExtractLiteralsPass(ac, fn).run_pass() CSE(ac, fn).run_pass(1, 5) assert ( diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 7970c4fc62..06e382fb76 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -55,11 +55,12 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: SimplifyCFGPass(ac, fn).run_pass() AlgebraicOptimizationPass(ac, fn).run_pass() BranchOptimizationPass(ac, fn).run_pass() - CSE(ac, fn).run_pass(2, 10) RemoveUnusedVariablesPass(ac, fn).run_pass() StoreExpansionPass(ac, fn).run_pass() + CSE(ac, fn).run_pass(2, 10) + RemoveUnusedVariablesPass(ac, fn).run_pass() DFTPass(ac, fn).run_pass() From 7d5b6aadc10bc7dab2f505bc5cc6b4487b01f862 Mon Sep 17 00:00:00 2001 From: Hodan Date: Sun, 6 Oct 2024 14:17:27 +0200 Subject: [PATCH 26/59] used new effects data structure to reduce duplication of code --- .../test_common_subexpression_elimination.py | 31 +++++++ vyper/venom/analysis/available_expression.py | 90 +++++++------------ vyper/venom/effects.py | 2 +- 3 files changed, 66 insertions(+), 57 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 5ce8fc367f..ddf9720a2e 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -3,6 +3,7 @@ from vyper.venom.analysis.analysis import IRAnalysesCache from vyper.venom.context import IRContext from vyper.venom.passes.common_subexpression_elimination import CSE +from vyper.venom.passes.store_expansion import StoreExpansionPass def test_common_subexpression_elimination(): @@ -78,6 +79,8 @@ def test_common_subexpression_elimination_effect_mstore(): bb.append_instruction("stop") ac = IRAnalysesCache(fn) + + StoreExpansionPass(ac, fn).run_pass() CSE(ac, fn).run_pass(1, 5) assert ( @@ -86,3 +89,31 @@ def test_common_subexpression_elimination_effect_mstore(): assert ( sum(1 for inst in bb.instructions if inst.opcode == "mload") == 1 ), "wrong number of mloads" + + +def test_common_subexpression_elimination_effect_mstore_with_msize(): + ctx = IRContext() + fn = ctx.create_function("test") + bb = fn.get_basic_block() + op = bb.append_instruction("store", 10) + bb.append_instruction("mstore", op, 0) + mload_1 = bb.append_instruction("mload", 0) + op = bb.append_instruction("store", 10) + bb.append_instruction("mstore", op, 0) + mload_2 = bb.append_instruction("mload", 0) + msize_read = bb.append_instruction("msize") + bb.append_instruction("add", mload_1, msize_read) + bb.append_instruction("add", mload_2, msize_read) + bb.append_instruction("stop") + + ac = IRAnalysesCache(fn) + + StoreExpansionPass(ac, fn).run_pass() + CSE(ac, fn).run_pass(1, 5) + + assert ( + sum(1 for inst in bb.instructions if inst.opcode == "mstore") == 2 + ), "wrong number of mstores" + assert ( + sum(1 for inst in bb.instructions if inst.opcode == "mload") == 2 + ), "wrong number of mloads" diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 5e767f6ad9..7d5f995d34 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -13,6 +13,7 @@ IRVariable, ) from vyper.venom.context import IRFunction +from vyper.venom.effects import EMPTY, Effects _MAX_DEPTH = 5 _MIN_DEPTH = 2 @@ -76,22 +77,23 @@ def get_depth(self) -> int: max_depth = d return max_depth + 1 - def get_effects(self) -> list[str]: - tmp_effects: set[str] = set(reads.get(self.opcode, ())) - tmp_effects = tmp_effects.union(writes.get(self.opcode, ())) + def get_reads(self, ignore_msize: bool) -> Effects: + tmp_reads = self.first_inst.get_read_effects() for op in self.operands: - if isinstance(op, IRVariable): - return list(_ALL) if isinstance(op, _Expression): - tmp_effects = tmp_effects.union(op.get_effects()) - return list(tmp_effects) + tmp_reads = tmp_reads | op.get_reads(ignore_msize) + if ignore_msize: + tmp_reads &= ~Effects.MSIZE + return tmp_reads - def get_reads(self) -> list[str]: - tmp_reads: set[str] = set(reads.get(self.opcode, ())) + def get_writes(self, ignore_msize: bool) -> Effects: + tmp_reads = self.first_inst.get_write_effects() for op in self.operands: if isinstance(op, _Expression): - tmp_reads = tmp_reads.union(op.get_reads()) - return list(tmp_reads) + tmp_reads = tmp_reads | op.get_writes(ignore_msize) + if ignore_msize: + tmp_reads &= ~Effects.MSIZE + return tmp_reads class _BBLattice: @@ -109,46 +111,6 @@ def __init__(self, bb: IRBasicBlock): _UNINTERESTING_OPCODES = ["store", "param", "offset", "phi", "nop"] -_ALL = ("storage", "transient", "memory", "immutables", "balance", "returndata", "log") - -writes = { - "sstore": ("storage",), - "tstore": ("transient",), - "mstore": ("memory",), - "istore": ("immutables",), - "call": _ALL, - "delegatecall": _ALL, - "staticcall": ("memory", "returndata"), - "create": _ALL, - "create2": _ALL, - "invoke": _ALL, # could be smarter, look up the effects of the invoked function - "dloadbytes": ("memory",), - "returndatacopy": ("memory",), - "calldatacopy": ("memory",), - "codecopy": ("memory",), - "extcodecopy": ("memory",), - "mcopy": ("memory",), - "log": ("log",), -} -reads = { - "sload": ("storage",), - "tload": ("transient",), - "iload": ("immutables",), - "mload": ("memory",), - "mcopy": ("memory",), - "call": _ALL, - "delegatecall": _ALL, - "staticcall": _ALL, - "returndatasize": ("returndata",), - "returndatacopy": ("returndata",), - "balance": ("balance",), - "selfbalance": ("balance",), - "log": ("memory", "log"), # I think here about log as a append to a log - "revert": ("memory",), - "return": ("memory",), - "sha3": ("memory",), -} - class _FunctionLattice: data: dict[IRBasicBlock, _BBLattice] @@ -166,6 +128,7 @@ class AvailableExpressionAnalysis(IRAnalysis): lattice: _FunctionLattice min_depth: int max_depth: int + ignore_msize: bool def __init__( self, @@ -185,6 +148,8 @@ def __init__( self.lattice = _FunctionLattice(function) + self.ignore_msize = not self._contains_msize() + def analyze(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): self.min_depth = min_depth self.max_depth = max_depth @@ -199,6 +164,13 @@ def analyze(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): if out not in worklist: worklist.append(out) + def _contains_msize(self) -> bool: + for bb in self.function.get_basic_blocks(): + for inst in bb.instructions: + if inst.opcode == "msize": + return True + return False + def _handle_bb(self, bb: IRBasicBlock) -> bool: available_expr: OrderedSet[_Expression] = OrderedSet() if len(bb.cfg_in) > 0: @@ -219,14 +191,20 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: change |= True inst_expr = self.get_expression(inst, available_expr) - write_effects = writes.get(inst_expr.opcode, ()) + # write_effects = inst.get_write_effects() # writes.get(inst_expr.opcode, ()) + write_effects = inst_expr.get_writes(self.ignore_msize) for expr in available_expr.copy(): - read_effects = expr.get_effects() - if any(eff in write_effects for eff in read_effects): + read_effects = expr.get_reads(self.ignore_msize) + if read_effects & write_effects != EMPTY: + available_expr.remove(expr) + continue + write_effects_expr = expr.get_writes(self.ignore_msize) + if write_effects_expr & write_effects != EMPTY: available_expr.remove(expr) - if inst_expr.get_depth() in range(self.min_depth, self.max_depth + 1) and not any( - eff in write_effects for eff in inst_expr.get_reads() + if ( + inst_expr.get_depth() in range(self.min_depth, self.max_depth + 1) + and write_effects & inst_expr.get_reads(self.ignore_msize) == EMPTY ): available_expr.add(inst_expr) diff --git a/vyper/venom/effects.py b/vyper/venom/effects.py index 20cc0e4b02..d04b75d2b6 100644 --- a/vyper/venom/effects.py +++ b/vyper/venom/effects.py @@ -64,7 +64,7 @@ class Effects(Flag): "selfbalance": BALANCE, "extcodecopy": EXTCODE, "selfdestruct": BALANCE, # may modify code, but after the transaction - "log": MEMORY, + "log": MEMORY | LOG, "revert": MEMORY, "return": MEMORY, "sha3": MEMORY, From 2de7731ea9e2d8bae84fe33a0d0c9b2ecc5c1a26 Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Thu, 10 Oct 2024 13:03:14 +0300 Subject: [PATCH 27/59] add test for commutative instructions --- .../test_common_subexpression_elimination.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index ddf9720a2e..0afb9b3edc 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -25,6 +25,25 @@ def test_common_subexpression_elimination(): assert sum(1 for inst in bb.instructions if inst.opcode == "mul") == 1, "wrong number of muls" +def test_common_subexpression_elimination_commutative(): + ctx = IRContext() + fn = ctx.create_function("test") + bb = fn.get_basic_block() + op = bb.append_instruction("store", 10) + sum_1 = bb.append_instruction("add", 10, op) + bb.append_instruction("mul", sum_1, 10) + sum_2 = bb.append_instruction("add", op, 10) + bb.append_instruction("mul", sum_2, 10) + bb.append_instruction("stop") + + ac = IRAnalysesCache(fn) + + CSE(ac, fn).run_pass(1, 5) + + assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 1, "wrong number of adds" + assert sum(1 for inst in bb.instructions if inst.opcode == "mul") == 1, "wrong number of muls" + + def test_common_subexpression_elimination_effects_1(): ctx = IRContext() fn = ctx.create_function("test") From 9efaea1daa973147c0f42182d12c3b686163321b Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Thu, 10 Oct 2024 13:04:41 +0300 Subject: [PATCH 28/59] simplification of `same()` and replacement by `__eq__` --- vyper/venom/analysis/available_expression.py | 39 ++++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 7d5f995d34..ca381f6b87 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -14,6 +14,7 @@ ) from vyper.venom.context import IRFunction from vyper.venom.effects import EMPTY, Effects +from vyper.venom.venom_to_assembly import COMMUTATIVE_INSTRUCTIONS _MAX_DEPTH = 5 _MIN_DEPTH = 2 @@ -26,9 +27,23 @@ class _Expression: operands: list["IROperand | _Expression"] def __eq__(self, other): - if not isinstance(other, _Expression): + if type(self) is not type(other): return False - return self.first_inst == other.first_inst + + if self.opcode != other.opcode: + return False + + # Early return special case for commutative instructions + if self.opcode in COMMUTATIVE_INSTRUCTIONS: + if self.operands[0] == other.operands[1] and self.operands[1] == other.operands[0]: + return True + + # General case + for self_op, other_op in zip(self.operands, other.operands): + if self_op != other_op: + return False + + return True def __hash__(self) -> int: return hash((self.opcode, *self.operands)) @@ -43,23 +58,6 @@ def __repr__(self) -> str: res += "]" return res - def same(self, other: "_Expression") -> bool: - if self.opcode != other.opcode: - return False - for self_op, other_op in zip(self.operands, other.operands): - if type(self_op) is not type(other_op): - return False - if isinstance(self_op, _Expression): - assert isinstance(other_op, _Expression) - if not self_op.same(other_op): - return False - else: - assert isinstance(self_op, IROperand) - assert isinstance(other_op, IROperand) - if self_op != other_op: - return False - return True - def contains_expr(self, expr: "_Expression") -> bool: for op in self.operands: if op == expr: @@ -241,8 +239,9 @@ def get_expression( depth = self.max_depth operands: list[IROperand | _Expression] = self._get_operands(inst, available_exprs, depth) expr = _Expression(inst, inst.opcode, operands) + for e in available_exprs: - if expr.same(e): + if expr == e: return e return expr From 43396978e10c660bc01fd0d90778322b6f6d3cee Mon Sep 17 00:00:00 2001 From: plodeada Date: Thu, 10 Oct 2024 13:50:17 +0200 Subject: [PATCH 29/59] different branches test --- .../test_common_subexpression_elimination.py | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 0afb9b3edc..c448d4d285 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -4,6 +4,8 @@ from vyper.venom.context import IRContext from vyper.venom.passes.common_subexpression_elimination import CSE from vyper.venom.passes.store_expansion import StoreExpansionPass +from vyper.venom.basicblock import IRBasicBlock +from vyper.venom.basicblock import IRLabel def test_common_subexpression_elimination(): @@ -136,3 +138,44 @@ def test_common_subexpression_elimination_effect_mstore_with_msize(): assert ( sum(1 for inst in bb.instructions if inst.opcode == "mload") == 2 ), "wrong number of mloads" + +def test_common_subexpression_elimination_different_branches(): + ctx = IRContext() + fn = ctx.create_function("test") + bb = fn.get_basic_block() + addr = bb.append_instruction("store", 10) + rand_cond = bb.append_instruction("mload", addr) + + br1 = IRBasicBlock(IRLabel("br1"), fn) + fn.append_basic_block(br1) + br2 = IRBasicBlock(IRLabel("br2"), fn) + fn.append_basic_block(br2) + join_bb = IRBasicBlock(IRLabel("join_bb"), fn) + fn.append_basic_block(join_bb) + + bb.append_instruction("jnz", rand_cond, br1.label, br2.label) + + def do_same(bb: IRBasicBlock, rand: int): + a = bb.append_instruction("store", 10) + b = bb.append_instruction("store", 20) + c = bb.append_instruction("add", a, b) + bb.append_instruction("mul", c, rand) + + do_same(br1, 1) + br1.append_instruction("jmp", join_bb.label) + do_same(br2, 2) + br2.append_instruction("jmp", join_bb.label) + do_same(join_bb, 3) + join_bb.append_instruction("stop") + + ac = IRAnalysesCache(fn) + + StoreExpansionPass(ac, fn).run_pass() + CSE(ac, fn).run_pass(1, 5) + + print(fn) + + assert sum(1 for inst in br1.instructions if inst.opcode == "add") == 1, "wrong number of adds" + assert sum(1 for inst in br2.instructions if inst.opcode == "add") == 1, "wrong number of adds" + assert sum(1 for inst in join_bb.instructions if inst.opcode == "add") == 1, "wrong number of adds" + From 8f767302f9871d2d57544bd6a7f4820ee32ee962 Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Thu, 10 Oct 2024 17:37:32 +0300 Subject: [PATCH 30/59] cleanup tests --- .../venom/test_common_subexpression_elimination.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index c448d4d285..62b7093290 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -1,11 +1,10 @@ import pytest from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.basicblock import IRBasicBlock, IRLabel from vyper.venom.context import IRContext from vyper.venom.passes.common_subexpression_elimination import CSE from vyper.venom.passes.store_expansion import StoreExpansionPass -from vyper.venom.basicblock import IRBasicBlock -from vyper.venom.basicblock import IRLabel def test_common_subexpression_elimination(): @@ -139,13 +138,14 @@ def test_common_subexpression_elimination_effect_mstore_with_msize(): sum(1 for inst in bb.instructions if inst.opcode == "mload") == 2 ), "wrong number of mloads" + def test_common_subexpression_elimination_different_branches(): ctx = IRContext() fn = ctx.create_function("test") bb = fn.get_basic_block() addr = bb.append_instruction("store", 10) rand_cond = bb.append_instruction("mload", addr) - + br1 = IRBasicBlock(IRLabel("br1"), fn) fn.append_basic_block(br1) br2 = IRBasicBlock(IRLabel("br2"), fn) @@ -173,9 +173,8 @@ def do_same(bb: IRBasicBlock, rand: int): StoreExpansionPass(ac, fn).run_pass() CSE(ac, fn).run_pass(1, 5) - print(fn) - assert sum(1 for inst in br1.instructions if inst.opcode == "add") == 1, "wrong number of adds" assert sum(1 for inst in br2.instructions if inst.opcode == "add") == 1, "wrong number of adds" - assert sum(1 for inst in join_bb.instructions if inst.opcode == "add") == 1, "wrong number of adds" - + assert ( + sum(1 for inst in join_bb.instructions if inst.opcode == "add") == 1 + ), "wrong number of adds" From 30ed887e601fbefdae56a0d77daf2b348fc8dc28 Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Thu, 10 Oct 2024 17:37:52 +0300 Subject: [PATCH 31/59] add `same()` to `IROperand` --- vyper/venom/basicblock.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/vyper/venom/basicblock.py b/vyper/venom/basicblock.py index 799dcfb33b..34c9726225 100644 --- a/vyper/venom/basicblock.py +++ b/vyper/venom/basicblock.py @@ -124,6 +124,11 @@ def __eq__(self, other) -> bool: return False return self.value == other.value + def same(self, other) -> bool: + if not isinstance(other, type(self)): + return False + return self.value == other.value + def __repr__(self) -> str: return str(self.value) From 2d33dc78dbde4a02d37bd4e1705dffcbab37c973 Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Thu, 10 Oct 2024 17:38:08 +0300 Subject: [PATCH 32/59] fix equality of expressions --- vyper/venom/analysis/available_expression.py | 24 +++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index ca381f6b87..e391f3bae3 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -26,7 +26,15 @@ class _Expression: opcode: str operands: list["IROperand | _Expression"] - def __eq__(self, other): + # equality for lattices only based on first_inst + def __eq__(self, other) -> bool: + if not isinstance(other, _Expression): + return False + + return self.first_inst == other.first_inst + + # Full equality for expressions based on opcode and operands + def same(self, other) -> bool: if type(self) is not type(other): return False @@ -35,12 +43,14 @@ def __eq__(self, other): # Early return special case for commutative instructions if self.opcode in COMMUTATIVE_INSTRUCTIONS: - if self.operands[0] == other.operands[1] and self.operands[1] == other.operands[0]: + if self.operands[0].same(other.operands[1]) and self.operands[1].same( + other.operands[0] + ): return True # General case for self_op, other_op in zip(self.operands, other.operands): - if self_op != other_op: + if not self_op.same(other_op): return False return True @@ -233,15 +243,13 @@ def get_expression( available_exprs: OrderedSet[_Expression] | None = None, depth: int | None = None, ) -> _Expression: - if available_exprs is None: - available_exprs = self.lattice.data[inst.parent].data[inst] - if depth is None: - depth = self.max_depth + available_exprs = available_exprs or self.lattice.data[inst.parent].data[inst] + depth = self.max_depth if depth is None else depth operands: list[IROperand | _Expression] = self._get_operands(inst, available_exprs, depth) expr = _Expression(inst, inst.opcode, operands) for e in available_exprs: - if expr == e: + if expr.same(e): return e return expr From 145ee3145f4dafc90802a65b00b88f4864e9987e Mon Sep 17 00:00:00 2001 From: Hodan Date: Thu, 10 Oct 2024 21:19:40 +0200 Subject: [PATCH 33/59] comments and fix of the hash function so it should hold that if x and y are same then x and y have same hash --- vyper/venom/analysis/available_expression.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index e391f3bae3..42be8c4b92 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -56,7 +56,7 @@ def same(self, other) -> bool: return True def __hash__(self) -> int: - return hash((self.opcode, *self.operands)) + return hash(self.first_inst) def __repr__(self) -> str: if self.opcode == "store": @@ -107,6 +107,9 @@ def get_writes(self, ignore_msize: bool) -> Effects: class _BBLattice: data: dict[IRInstruction, OrderedSet[_Expression]] out: OrderedSet[_Expression] + + # used to check if the basic block has to be + # recalculated in_cache: OrderedSet[_Expression] | None def __init__(self, bb: IRBasicBlock): @@ -134,6 +137,9 @@ class AvailableExpressionAnalysis(IRAnalysis): inst_to_expr: dict[IRInstruction, _Expression] = dict() dfg: DFGAnalysis lattice: _FunctionLattice + + # the size of the expressions + # that are considered in the analysis min_depth: int max_depth: int ignore_msize: bool @@ -161,6 +167,7 @@ def __init__( def analyze(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): self.min_depth = min_depth self.max_depth = max_depth + worklist: deque = deque() worklist.append(self.function.entry) while len(worklist) > 0: @@ -199,7 +206,6 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: change |= True inst_expr = self.get_expression(inst, available_expr) - # write_effects = inst.get_write_effects() # writes.get(inst_expr.opcode, ()) write_effects = inst_expr.get_writes(self.ignore_msize) for expr in available_expr.copy(): read_effects = expr.get_reads(self.ignore_msize) From c6a825a1908c15d112fc9877ddbfa3d6b213396c Mon Sep 17 00:00:00 2001 From: Hodan Date: Thu, 10 Oct 2024 21:46:27 +0200 Subject: [PATCH 34/59] lint --- vyper/venom/passes/common_subexpression_elimination.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 6f47fa0215..4f0152043f 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -41,7 +41,7 @@ def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: res: dict[IRInstruction, IRInstruction] = dict() for bb in self.function.get_basic_blocks(): for inst in bb.instructions: - # skip instruction that for sure + # skip instruction that for sure # wont be substituted if inst in _UNINTERESTING_OPCODES: continue From 674577aa21d2b7609a15018f659c80226bad91a1 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 11 Oct 2024 10:19:29 +0200 Subject: [PATCH 35/59] comments and removed some unnecessery code --- vyper/venom/analysis/available_expression.py | 24 +++++++++---------- .../common_subexpression_elimination.py | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 42be8c4b92..9cddb16271 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -19,6 +19,7 @@ _MAX_DEPTH = 5 _MIN_DEPTH = 2 +UNINTERESTING_OPCODES = ["store", "param", "offset", "phi", "nop"] @dataclass class _Expression: @@ -108,21 +109,13 @@ class _BBLattice: data: dict[IRInstruction, OrderedSet[_Expression]] out: OrderedSet[_Expression] - # used to check if the basic block has to be - # recalculated - in_cache: OrderedSet[_Expression] | None - def __init__(self, bb: IRBasicBlock): self.data = dict() self.out = OrderedSet() - self.in_cache = None for inst in bb.instructions: self.data[inst] = OrderedSet() -_UNINTERESTING_OPCODES = ["store", "param", "offset", "phi", "nop"] - - class _FunctionLattice: data: dict[IRBasicBlock, _BBLattice] @@ -179,6 +172,10 @@ def analyze(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): if out not in worklist: worklist.append(out) + # msize effect should be only necessery + # to be handled when there is a possibility + # of msize read otherwise it should not make difference + # for this analysis def _contains_msize(self) -> bool: for bb in self.function.get_basic_blocks(): for inst in bb.instructions: @@ -194,16 +191,12 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: ) bb_lat = self.lattice.data[bb] - if bb_lat.in_cache is not None and available_expr == bb_lat.in_cache: - return False - bb_lat.in_cache = available_expr change = False for inst in bb.instructions: - if inst.opcode in _UNINTERESTING_OPCODES or inst.opcode in BB_TERMINATORS: + if inst.opcode in UNINTERESTING_OPCODES or inst.opcode in BB_TERMINATORS: continue if available_expr != bb_lat.data[inst]: bb_lat.data[inst] = available_expr.copy() - change |= True inst_expr = self.get_expression(inst, available_expr) write_effects = inst_expr.get_writes(self.ignore_msize) @@ -224,6 +217,8 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: if available_expr != bb_lat.out: bb_lat.out = available_expr.copy() + # change is only necessery when the output of the + # basic block is changed (otherwise it wont affect rest) change |= True return change @@ -234,6 +229,9 @@ def _get_operand( if depth > 0 and isinstance(op, IRVariable): inst = self.dfg.get_producing_instruction(op) assert inst is not None + # this can both create better solutions and is necessery + # for correct effect handle, otherwise you could go over + # effect bounderies if not inst.is_volatile: return self.get_expression(inst, available_exprs, depth - 1) return op diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 4f0152043f..e362a46554 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -1,6 +1,6 @@ from vyper.utils import OrderedSet from vyper.venom.analysis.available_expression import ( - _UNINTERESTING_OPCODES, + UNINTERESTING_OPCODES, AvailableExpressionAnalysis, ) from vyper.venom.analysis.dfg import DFGAnalysis @@ -43,7 +43,7 @@ def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: for inst in bb.instructions: # skip instruction that for sure # wont be substituted - if inst in _UNINTERESTING_OPCODES: + if inst in UNINTERESTING_OPCODES: continue inst_expr = self.available_expression_analysis.get_expression(inst) avail = self.available_expression_analysis.get_available(inst) From 479271e2b1fa1aa5129018b69d0395a427d5cebd Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 11 Oct 2024 10:26:45 +0200 Subject: [PATCH 36/59] lint --- vyper/venom/analysis/available_expression.py | 1 + 1 file changed, 1 insertion(+) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 9cddb16271..5be8c81ce1 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -21,6 +21,7 @@ UNINTERESTING_OPCODES = ["store", "param", "offset", "phi", "nop"] + @dataclass class _Expression: first_inst: IRInstruction From 47e0a0ef946bf56f2e4bede2b971ba50f9933e3c Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 14 Oct 2024 09:48:32 +0200 Subject: [PATCH 37/59] added test for sanity check on effects and removed the change in the DFT pass --- .../test_common_subexpression_elimination.py | 18 ++++++++++++++++++ vyper/venom/passes/dft.py | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 62b7093290..3535a202f3 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -84,6 +84,24 @@ def test_common_subexpression_elimination_effects_2(): assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" +def test_common_subexpression_elimination_effects_3(): + ctx = IRContext() + fn = ctx.create_function("test") + bb = fn.get_basic_block() + addr1 = bb.append_instruction("store", 10) + addr2 = bb.append_instruction("store", 10) + bb.append_instruction("mstore", 0, addr1) + bb.append_instruction("mstore", 2, addr2) + bb.append_instruction("mstore", 0, addr1) + bb.append_instruction("stop") + + ac = IRAnalysesCache(fn) + + CSE(ac, fn).run_pass() + + assert sum(1 for inst in bb.instructions if inst.opcode == "mstore") == 3, "wrong number of mstores" + + def test_common_subexpression_elimination_effect_mstore(): ctx = IRContext() diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index 2c8e53f2a6..f45a60079c 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -46,7 +46,7 @@ def _process_instruction_r(self, bb: IRBasicBlock, inst: IRInstruction, offset: if target.parent != inst.parent or target.fence_id != inst.fence_id: # don't reorder across basic block or fence boundaries continue - self._process_instruction_r(bb, target, 0) + self._process_instruction_r(bb, target, offset) self.inst_order[inst] = self.inst_order_num + offset From 6e70e6e35288731079eb0940cbd35a1983823438 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 14 Oct 2024 10:15:37 +0200 Subject: [PATCH 38/59] fix after merge --- vyper/venom/passes/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/vyper/venom/passes/__init__.py b/vyper/venom/passes/__init__.py index 83098234c1..743707a93b 100644 --- a/vyper/venom/passes/__init__.py +++ b/vyper/venom/passes/__init__.py @@ -9,3 +9,4 @@ from .simplify_cfg import SimplifyCFGPass from .store_elimination import StoreElimination from .store_expansion import StoreExpansionPass +from .common_subexpression_elimination import CSE From f244a136587b735f542ce23d890af4d32073da7e Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 14 Oct 2024 10:17:23 +0200 Subject: [PATCH 39/59] fix after merge --- .../compiler/venom/test_common_subexpression_elimination.py | 6 ++++-- vyper/venom/__init__.py | 2 +- vyper/venom/passes/__init__.py | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 3535a202f3..a32d1ad39f 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -84,6 +84,7 @@ def test_common_subexpression_elimination_effects_2(): assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" + def test_common_subexpression_elimination_effects_3(): ctx = IRContext() fn = ctx.create_function("test") @@ -99,8 +100,9 @@ def test_common_subexpression_elimination_effects_3(): CSE(ac, fn).run_pass() - assert sum(1 for inst in bb.instructions if inst.opcode == "mstore") == 3, "wrong number of mstores" - + assert ( + sum(1 for inst in bb.instructions if inst.opcode == "mstore") == 3 + ), "wrong number of mstores" def test_common_subexpression_elimination_effect_mstore(): diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 7ac1f6cd3a..635c3464ab 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -10,6 +10,7 @@ from vyper.venom.function import IRFunction from vyper.venom.ir_node_to_venom import ir_node_to_venom from vyper.venom.passes import ( + CSE, SCCP, AlgebraicOptimizationPass, BranchOptimizationPass, @@ -20,7 +21,6 @@ SimplifyCFGPass, StoreElimination, StoreExpansionPass, - CSE, ) from vyper.venom.venom_to_assembly import VenomCompiler diff --git a/vyper/venom/passes/__init__.py b/vyper/venom/passes/__init__.py index 743707a93b..2d48e81f5b 100644 --- a/vyper/venom/passes/__init__.py +++ b/vyper/venom/passes/__init__.py @@ -1,5 +1,6 @@ from .algebraic_optimization import AlgebraicOptimizationPass from .branch_optimization import BranchOptimizationPass +from .common_subexpression_elimination import CSE from .dft import DFTPass from .make_ssa import MakeSSA from .mem2var import Mem2Var @@ -9,4 +10,3 @@ from .simplify_cfg import SimplifyCFGPass from .store_elimination import StoreElimination from .store_expansion import StoreExpansionPass -from .common_subexpression_elimination import CSE From f3b709b18a600d8d8bf590267d92f122cb3843a8 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 14 Oct 2024 10:23:18 +0200 Subject: [PATCH 40/59] fixed circular import from commutative instruction --- vyper/venom/analysis/available_expression.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 5be8c81ce1..1bd9e60402 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -14,7 +14,8 @@ ) from vyper.venom.context import IRFunction from vyper.venom.effects import EMPTY, Effects -from vyper.venom.venom_to_assembly import COMMUTATIVE_INSTRUCTIONS + +COMMUTATIVE_INSTRUCTIONS = frozenset(["add", "mul", "smul", "or", "xor", "and", "eq"]) _MAX_DEPTH = 5 _MIN_DEPTH = 2 From b9caf57e24a8952bad7321ab13c7519566d41995 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 15 Oct 2024 08:53:25 +0200 Subject: [PATCH 41/59] idempotent instruction better support and test for the logs --- .../test_common_subexpression_elimination.py | 35 ++++++++++++++++++- vyper/venom/analysis/available_expression.py | 7 ++-- vyper/venom/effects.py | 2 +- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index a32d1ad39f..46b22b7560 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -85,6 +85,40 @@ def test_common_subexpression_elimination_effects_2(): assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" +def test_common_subexpression_elimination_logs(): + ctx = IRContext() + fn = ctx.create_function("test") + bb = fn.get_basic_block() + num2 = bb.append_instruction("store", 10) + num1 = bb.append_instruction("store", 20) + num3 = bb.append_instruction("store", 20) + bb.append_instruction("log", num1) + bb.append_instruction("log", num2) + bb.append_instruction("log", num1) + bb.append_instruction("log", num3) + bb.append_instruction("stop") + + ac = IRAnalysesCache(fn) + + print(fn) + + from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis + + avail: AvailableExpressionAnalysis = ac.request_analysis(AvailableExpressionAnalysis) + + print() + + for bb in fn.get_basic_blocks(): + for inst in bb.instructions: + print(avail.get_available(inst)) + + CSE(ac, fn).run_pass() + print(fn) + + assert ( + sum(1 for inst in bb.instructions if inst.opcode == "log") == 4 + ), "wrong number of log" + def test_common_subexpression_elimination_effects_3(): ctx = IRContext() fn = ctx.create_function("test") @@ -104,7 +138,6 @@ def test_common_subexpression_elimination_effects_3(): sum(1 for inst in bb.instructions if inst.opcode == "mstore") == 3 ), "wrong number of mstores" - def test_common_subexpression_elimination_effect_mstore(): ctx = IRContext() fn = ctx.create_function("test") diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 1bd9e60402..82f9c563dd 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -21,6 +21,7 @@ _MIN_DEPTH = 2 UNINTERESTING_OPCODES = ["store", "param", "offset", "phi", "nop"] +_IDEMPOTENT_INSTRUCTIONS = ["log", "call", "staticcall", "delegatecall", "invoke"] @dataclass @@ -87,7 +88,7 @@ def get_depth(self) -> int: if d > max_depth: max_depth = d return max_depth + 1 - + def get_reads(self, ignore_msize: bool) -> Effects: tmp_reads = self.first_inst.get_read_effects() for op in self.operands: @@ -197,9 +198,9 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: for inst in bb.instructions: if inst.opcode in UNINTERESTING_OPCODES or inst.opcode in BB_TERMINATORS: continue + if available_expr != bb_lat.data[inst]: bb_lat.data[inst] = available_expr.copy() - inst_expr = self.get_expression(inst, available_expr) write_effects = inst_expr.get_writes(self.ignore_msize) for expr in available_expr.copy(): @@ -213,10 +214,12 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: if ( inst_expr.get_depth() in range(self.min_depth, self.max_depth + 1) + and inst.opcode not in _IDEMPOTENT_INSTRUCTIONS and write_effects & inst_expr.get_reads(self.ignore_msize) == EMPTY ): available_expr.add(inst_expr) + if available_expr != bb_lat.out: bb_lat.out = available_expr.copy() # change is only necessery when the output of the diff --git a/vyper/venom/effects.py b/vyper/venom/effects.py index d04b75d2b6..20cc0e4b02 100644 --- a/vyper/venom/effects.py +++ b/vyper/venom/effects.py @@ -64,7 +64,7 @@ class Effects(Flag): "selfbalance": BALANCE, "extcodecopy": EXTCODE, "selfdestruct": BALANCE, # may modify code, but after the transaction - "log": MEMORY | LOG, + "log": MEMORY, "revert": MEMORY, "return": MEMORY, "sha3": MEMORY, From ee7a293c30924436f4ade93800844d024ba010ba Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 15 Oct 2024 08:54:56 +0200 Subject: [PATCH 42/59] test cleanup and lint --- .../test_common_subexpression_elimination.py | 19 +++---------------- vyper/venom/analysis/available_expression.py | 3 +-- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 46b22b7560..6cdbbc2ef6 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -100,24 +100,10 @@ def test_common_subexpression_elimination_logs(): ac = IRAnalysesCache(fn) - print(fn) - - from vyper.venom.analysis.available_expression import AvailableExpressionAnalysis - - avail: AvailableExpressionAnalysis = ac.request_analysis(AvailableExpressionAnalysis) - - print() - - for bb in fn.get_basic_blocks(): - for inst in bb.instructions: - print(avail.get_available(inst)) - CSE(ac, fn).run_pass() - print(fn) - assert ( - sum(1 for inst in bb.instructions if inst.opcode == "log") == 4 - ), "wrong number of log" + assert sum(1 for inst in bb.instructions if inst.opcode == "log") == 4, "wrong number of log" + def test_common_subexpression_elimination_effects_3(): ctx = IRContext() @@ -138,6 +124,7 @@ def test_common_subexpression_elimination_effects_3(): sum(1 for inst in bb.instructions if inst.opcode == "mstore") == 3 ), "wrong number of mstores" + def test_common_subexpression_elimination_effect_mstore(): ctx = IRContext() fn = ctx.create_function("test") diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 82f9c563dd..c7d1d197e5 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -88,7 +88,7 @@ def get_depth(self) -> int: if d > max_depth: max_depth = d return max_depth + 1 - + def get_reads(self, ignore_msize: bool) -> Effects: tmp_reads = self.first_inst.get_read_effects() for op in self.operands: @@ -219,7 +219,6 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: ): available_expr.add(inst_expr) - if available_expr != bb_lat.out: bb_lat.out = available_expr.copy() # change is only necessery when the output of the From 7ca27b694ad2a12b04c132b2e9ffdbe7e22b9beb Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 15 Oct 2024 20:31:04 +0200 Subject: [PATCH 43/59] used commutative --- vyper/venom/analysis/available_expression.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index c7d1d197e5..b704cb2edf 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -15,8 +15,6 @@ from vyper.venom.context import IRFunction from vyper.venom.effects import EMPTY, Effects -COMMUTATIVE_INSTRUCTIONS = frozenset(["add", "mul", "smul", "or", "xor", "and", "eq"]) - _MAX_DEPTH = 5 _MIN_DEPTH = 2 @@ -46,7 +44,7 @@ def same(self, other) -> bool: return False # Early return special case for commutative instructions - if self.opcode in COMMUTATIVE_INSTRUCTIONS: + if self.is_commutative: if self.operands[0].same(other.operands[1]) and self.operands[1].same( other.operands[0] ): @@ -107,6 +105,10 @@ def get_writes(self, ignore_msize: bool) -> Effects: tmp_reads &= ~Effects.MSIZE return tmp_reads + @property + def is_commutative(self) -> bool: + return self.first_inst.is_commutative + class _BBLattice: data: dict[IRInstruction, OrderedSet[_Expression]] From 74aab9224f651462aedf16f55de08db55129b6a9 Mon Sep 17 00:00:00 2001 From: Hodan Date: Wed, 16 Oct 2024 16:48:08 +0200 Subject: [PATCH 44/59] caching of read and writes effects --- vyper/venom/analysis/available_expression.py | 26 +++++++++++--------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index b704cb2edf..b7c5653ec5 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -1,5 +1,6 @@ from collections import deque from dataclasses import dataclass +from functools import cached_property from vyper.utils import OrderedSet from vyper.venom.analysis.analysis import IRAnalysesCache, IRAnalysis @@ -27,6 +28,7 @@ class _Expression: first_inst: IRInstruction opcode: str operands: list["IROperand | _Expression"] + ignore_msize: bool # equality for lattices only based on first_inst def __eq__(self, other) -> bool: @@ -87,21 +89,23 @@ def get_depth(self) -> int: max_depth = d return max_depth + 1 - def get_reads(self, ignore_msize: bool) -> Effects: + @cached_property + def get_reads(self) -> Effects: tmp_reads = self.first_inst.get_read_effects() for op in self.operands: if isinstance(op, _Expression): - tmp_reads = tmp_reads | op.get_reads(ignore_msize) - if ignore_msize: + tmp_reads = tmp_reads | op.get_reads + if self.ignore_msize: tmp_reads &= ~Effects.MSIZE return tmp_reads - def get_writes(self, ignore_msize: bool) -> Effects: + @cached_property + def get_writes(self) -> Effects: tmp_reads = self.first_inst.get_write_effects() for op in self.operands: if isinstance(op, _Expression): - tmp_reads = tmp_reads | op.get_writes(ignore_msize) - if ignore_msize: + tmp_reads = tmp_reads | op.get_writes + if self.ignore_msize: tmp_reads &= ~Effects.MSIZE return tmp_reads @@ -204,20 +208,20 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: if available_expr != bb_lat.data[inst]: bb_lat.data[inst] = available_expr.copy() inst_expr = self.get_expression(inst, available_expr) - write_effects = inst_expr.get_writes(self.ignore_msize) + write_effects = inst_expr.get_writes for expr in available_expr.copy(): - read_effects = expr.get_reads(self.ignore_msize) + read_effects = expr.get_reads if read_effects & write_effects != EMPTY: available_expr.remove(expr) continue - write_effects_expr = expr.get_writes(self.ignore_msize) + write_effects_expr = expr.get_writes if write_effects_expr & write_effects != EMPTY: available_expr.remove(expr) if ( inst_expr.get_depth() in range(self.min_depth, self.max_depth + 1) and inst.opcode not in _IDEMPOTENT_INSTRUCTIONS - and write_effects & inst_expr.get_reads(self.ignore_msize) == EMPTY + and write_effects & inst_expr.get_reads == EMPTY ): available_expr.add(inst_expr) @@ -256,7 +260,7 @@ def get_expression( available_exprs = available_exprs or self.lattice.data[inst.parent].data[inst] depth = self.max_depth if depth is None else depth operands: list[IROperand | _Expression] = self._get_operands(inst, available_exprs, depth) - expr = _Expression(inst, inst.opcode, operands) + expr = _Expression(inst, inst.opcode, operands, self.ignore_msize) for e in available_exprs: if expr.same(e): From 00b5e9e1eea00134bcf1f30ef800326e5f05b9ca Mon Sep 17 00:00:00 2001 From: plodeada Date: Fri, 18 Oct 2024 10:46:41 +0200 Subject: [PATCH 45/59] better same compare and caching of exprs and depth --- vyper/venom/analysis/available_expression.py | 27 ++++++++++++-------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index b7c5653ec5..4baab36a7b 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -1,4 +1,3 @@ -from collections import deque from dataclasses import dataclass from functools import cached_property @@ -45,6 +44,9 @@ def same(self, other) -> bool: if self.opcode != other.opcode: return False + if self.first_inst == other.first_inst: + return True + # Early return special case for commutative instructions if self.is_commutative: if self.operands[0].same(other.operands[1]) and self.operands[1].same( @@ -80,11 +82,12 @@ def contains_expr(self, expr: "_Expression") -> bool: return True return False + @cached_property def get_depth(self) -> int: max_depth = 0 for op in self.operands: if isinstance(op, _Expression): - d = op.get_depth() + d = op.get_depth if d > max_depth: max_depth = d return max_depth + 1 @@ -170,16 +173,15 @@ def analyze(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): self.min_depth = min_depth self.max_depth = max_depth - worklist: deque = deque() - worklist.append(self.function.entry) + worklist: OrderedSet = OrderedSet() + worklist.add(self.function.entry) while len(worklist) > 0: - bb: IRBasicBlock = worklist.popleft() + bb: IRBasicBlock = worklist.pop() changed = self._handle_bb(bb) if changed: for out in bb.cfg_out: - if out not in worklist: - worklist.append(out) + worklist.add(out) # msize effect should be only necessery # to be handled when there is a possibility @@ -219,7 +221,7 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: available_expr.remove(expr) if ( - inst_expr.get_depth() in range(self.min_depth, self.max_depth + 1) + inst_expr.get_depth in range(self.min_depth, self.max_depth + 1) and inst.opcode not in _IDEMPOTENT_INSTRUCTIONS and write_effects & inst_expr.get_reads == EMPTY ): @@ -242,8 +244,11 @@ def _get_operand( # this can both create better solutions and is necessery # for correct effect handle, otherwise you could go over # effect bounderies - if not inst.is_volatile: - return self.get_expression(inst, available_exprs, depth - 1) + if inst.is_volatile: + return op + if inst in self.inst_to_expr: + return self.inst_to_expr[inst] + return self.get_expression(inst, available_exprs, depth - 1) return op def _get_operands( @@ -264,8 +269,10 @@ def get_expression( for e in available_exprs: if expr.same(e): + self.inst_to_expr[inst] = e return e + self.inst_to_expr[inst] = expr return expr def get_available(self, inst: IRInstruction) -> OrderedSet[_Expression]: From 1b576f3798663d9abdd7b50cf3bff61b3f2d6cd1 Mon Sep 17 00:00:00 2001 From: plodeada Date: Fri, 18 Oct 2024 11:14:14 +0200 Subject: [PATCH 46/59] more caching --- vyper/venom/analysis/available_expression.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 4baab36a7b..d33d9ed2d9 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -267,6 +267,9 @@ def get_expression( operands: list[IROperand | _Expression] = self._get_operands(inst, available_exprs, depth) expr = _Expression(inst, inst.opcode, operands, self.ignore_msize) + if inst in self.inst_to_expr and self.inst_to_expr[inst] in available_exprs: + return self.inst_to_expr[inst] + for e in available_exprs: if expr.same(e): self.inst_to_expr[inst] = e From c99341ff732745ff392e3593235e5881dc14a018 Mon Sep 17 00:00:00 2001 From: plodeada Date: Fri, 18 Oct 2024 11:39:20 +0200 Subject: [PATCH 47/59] should be better reevaluating --- vyper/venom/passes/common_subexpression_elimination.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index e362a46554..3edddd017a 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -29,11 +29,8 @@ def run_pass(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): self._replace(replace_dict) self.analyses_cache.invalidate_analysis(DFGAnalysis) self.analyses_cache.invalidate_analysis(LivenessAnalysis) - available_expression_analysis = self.analyses_cache.force_analysis( - AvailableExpressionAnalysis, min_depth, max_depth - ) - assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) - self.available_expression_analysis = available_expression_analysis + # should be ok to be reevaluted + self.available_expression_analysis.analyze(min_depth, max_depth) # return instruction and to which instruction it could # replaced by From 3b3a5465605edbc7d4a8210dcfde706b228fe5fa Mon Sep 17 00:00:00 2001 From: plodeada Date: Fri, 18 Oct 2024 14:42:23 +0200 Subject: [PATCH 48/59] idempotent rename and small clean --- vyper/venom/analysis/available_expression.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index d33d9ed2d9..c20ec31fc8 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -19,7 +19,7 @@ _MIN_DEPTH = 2 UNINTERESTING_OPCODES = ["store", "param", "offset", "phi", "nop"] -_IDEMPOTENT_INSTRUCTIONS = ["log", "call", "staticcall", "delegatecall", "invoke"] +_NONIDEMPOTENT_INSTRUCTIONS = frozenset(["log", "call", "staticcall", "delegatecall", "invoke"]) @dataclass @@ -74,14 +74,6 @@ def __repr__(self) -> str: res += "]" return res - def contains_expr(self, expr: "_Expression") -> bool: - for op in self.operands: - if op == expr: - return True - if isinstance(op, _Expression) and op.contains_expr(expr): - return True - return False - @cached_property def get_depth(self) -> int: max_depth = 0 @@ -138,7 +130,6 @@ def __init__(self, function: IRFunction): class AvailableExpressionAnalysis(IRAnalysis): - expressions: OrderedSet[_Expression] = OrderedSet() inst_to_expr: dict[IRInstruction, _Expression] = dict() dfg: DFGAnalysis lattice: _FunctionLattice @@ -222,7 +213,7 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: if ( inst_expr.get_depth in range(self.min_depth, self.max_depth + 1) - and inst.opcode not in _IDEMPOTENT_INSTRUCTIONS + and inst.opcode not in _NONIDEMPOTENT_INSTRUCTIONS and write_effects & inst_expr.get_reads == EMPTY ): available_expr.add(inst_expr) From 3e45b314f32af5b79a10eaad58c66ad78994cbe2 Mon Sep 17 00:00:00 2001 From: plodeada Date: Fri, 18 Oct 2024 15:12:46 +0200 Subject: [PATCH 49/59] replaced lattice structure by just dicts to make it more explicit --- vyper/venom/analysis/available_expression.py | 45 ++++++-------------- 1 file changed, 14 insertions(+), 31 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index c20ec31fc8..cb10d6f43d 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -109,30 +109,11 @@ def is_commutative(self) -> bool: return self.first_inst.is_commutative -class _BBLattice: - data: dict[IRInstruction, OrderedSet[_Expression]] - out: OrderedSet[_Expression] - - def __init__(self, bb: IRBasicBlock): - self.data = dict() - self.out = OrderedSet() - for inst in bb.instructions: - self.data[inst] = OrderedSet() - - -class _FunctionLattice: - data: dict[IRBasicBlock, _BBLattice] - - def __init__(self, function: IRFunction): - self.data = dict() - for bb in function.get_basic_blocks(): - self.data[bb] = _BBLattice(bb) - - class AvailableExpressionAnalysis(IRAnalysis): - inst_to_expr: dict[IRInstruction, _Expression] = dict() + inst_to_expr: dict[IRInstruction, _Expression] dfg: DFGAnalysis - lattice: _FunctionLattice + inst_to_available: dict[IRInstruction, OrderedSet[_Expression]] + bb_outs: dict[IRBasicBlock, OrderedSet[_Expression]] # the size of the expressions # that are considered in the analysis @@ -156,7 +137,9 @@ def __init__( self.min_depth = min_depth self.max_depth = max_depth - self.lattice = _FunctionLattice(function) + self.inst_to_expr = dict() + self.inst_to_available = dict() + self.bb_outs = dict() self.ignore_msize = not self._contains_msize() @@ -189,17 +172,17 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: available_expr: OrderedSet[_Expression] = OrderedSet() if len(bb.cfg_in) > 0: available_expr = OrderedSet.intersection( - *(self.lattice.data[in_bb].out for in_bb in bb.cfg_in) + *(self.bb_outs.get(in_bb, OrderedSet()) for in_bb in bb.cfg_in) ) - bb_lat = self.lattice.data[bb] + #bb_lat = self.lattice.data[bb] change = False for inst in bb.instructions: if inst.opcode in UNINTERESTING_OPCODES or inst.opcode in BB_TERMINATORS: continue - if available_expr != bb_lat.data[inst]: - bb_lat.data[inst] = available_expr.copy() + if inst not in self.inst_to_available or available_expr != self.inst_to_available[inst]: + self.inst_to_available[inst] = available_expr.copy() inst_expr = self.get_expression(inst, available_expr) write_effects = inst_expr.get_writes for expr in available_expr.copy(): @@ -218,8 +201,8 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: ): available_expr.add(inst_expr) - if available_expr != bb_lat.out: - bb_lat.out = available_expr.copy() + if bb not in self.bb_outs or available_expr != self.bb_outs[bb]: + self.bb_outs[bb] = available_expr.copy() # change is only necessery when the output of the # basic block is changed (otherwise it wont affect rest) change |= True @@ -253,7 +236,7 @@ def get_expression( available_exprs: OrderedSet[_Expression] | None = None, depth: int | None = None, ) -> _Expression: - available_exprs = available_exprs or self.lattice.data[inst.parent].data[inst] + available_exprs = available_exprs or self.inst_to_available.get(inst, OrderedSet()) depth = self.max_depth if depth is None else depth operands: list[IROperand | _Expression] = self._get_operands(inst, available_exprs, depth) expr = _Expression(inst, inst.opcode, operands, self.ignore_msize) @@ -270,4 +253,4 @@ def get_expression( return expr def get_available(self, inst: IRInstruction) -> OrderedSet[_Expression]: - return self.lattice.data[inst.parent].data[inst] + return self.inst_to_available.get(inst, OrderedSet()) From 73bd8ee45b77cda03f9120dae8823ed72aa59176 Mon Sep 17 00:00:00 2001 From: plodeada Date: Fri, 18 Oct 2024 15:16:50 +0200 Subject: [PATCH 50/59] lint --- vyper/venom/analysis/available_expression.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index cb10d6f43d..53b0527339 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -175,7 +175,7 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: *(self.bb_outs.get(in_bb, OrderedSet()) for in_bb in bb.cfg_in) ) - #bb_lat = self.lattice.data[bb] + # bb_lat = self.lattice.data[bb] change = False for inst in bb.instructions: if inst.opcode in UNINTERESTING_OPCODES or inst.opcode in BB_TERMINATORS: @@ -237,6 +237,7 @@ def get_expression( depth: int | None = None, ) -> _Expression: available_exprs = available_exprs or self.inst_to_available.get(inst, OrderedSet()) + assert available_exprs is not None depth = self.max_depth if depth is None else depth operands: list[IROperand | _Expression] = self._get_operands(inst, available_exprs, depth) expr = _Expression(inst, inst.opcode, operands, self.ignore_msize) From b6e0048daa434e07f22590c5ca2a83d4eb762d27 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 21 Oct 2024 11:10:05 +0200 Subject: [PATCH 51/59] added small heuristic for small expressions --- vyper/venom/passes/common_subexpression_elimination.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 3edddd017a..0897e8dc3d 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -44,7 +44,11 @@ def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: continue inst_expr = self.available_expression_analysis.get_expression(inst) avail = self.available_expression_analysis.get_available(inst) - if inst_expr in avail: + # heuristic to not replace small expressions + # basic block bounderies (it can create better codesize) + if inst_expr in avail and ( + inst_expr.get_depth > 2 or inst.parent == inst_expr.first_inst.parent + ): res[inst] = inst_expr.first_inst return res From 45c361284c3ebc67596be54450a60ebc58354920 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 10 Nov 2024 13:05:38 +0100 Subject: [PATCH 52/59] add some review --- vyper/venom/analysis/available_expression.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 53b0527339..be2eb67585 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -1,3 +1,5 @@ +# REVIEW: rename this to cse_analysis or common_subexpression_analysis + from dataclasses import dataclass from functools import cached_property @@ -24,8 +26,11 @@ @dataclass class _Expression: + # REVIEW: rename to root_inst first_inst: IRInstruction opcode: str + # REVIEW: bad type: this list only contains _Expressions, otherwise + # we could not call same(self_op, other_op) in the recursion. operands: list["IROperand | _Expression"] ignore_msize: bool @@ -37,6 +42,11 @@ def __eq__(self, other) -> bool: return self.first_inst == other.first_inst # Full equality for expressions based on opcode and operands + # REVIEW: this is inefficient. we do not need to do the recursion if + # we do a shallow comparison - we can check `self_op is other_op` for + # in the loop over zip(self.operands, other.operands). because of + # how expressions are computed, we are guaranteed uniqueness of expressions + # in available_expressions def same(self, other) -> bool: if type(self) is not type(other): return False @@ -61,6 +71,8 @@ def same(self, other) -> bool: return True + # REVIEW: move this closer in the file to __eq__ because they are + # related functions def __hash__(self) -> int: return hash(self.first_inst) @@ -109,6 +121,7 @@ def is_commutative(self) -> bool: return self.first_inst.is_commutative +# REVIEW: rename to CSEAnalysis class AvailableExpressionAnalysis(IRAnalysis): inst_to_expr: dict[IRInstruction, _Expression] dfg: DFGAnalysis @@ -181,6 +194,7 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: if inst.opcode in UNINTERESTING_OPCODES or inst.opcode in BB_TERMINATORS: continue + # REVIEW: why replace inst_to_available if they are not equal? if inst not in self.inst_to_available or available_expr != self.inst_to_available[inst]: self.inst_to_available[inst] = available_expr.copy() inst_expr = self.get_expression(inst, available_expr) @@ -194,6 +208,8 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: if write_effects_expr & write_effects != EMPTY: available_expr.remove(expr) + # REVIEW: we should not care about depth if `same()` is not + # implemented recursively. if ( inst_expr.get_depth in range(self.min_depth, self.max_depth + 1) and inst.opcode not in _NONIDEMPOTENT_INSTRUCTIONS @@ -245,6 +261,7 @@ def get_expression( if inst in self.inst_to_expr and self.inst_to_expr[inst] in available_exprs: return self.inst_to_expr[inst] + # REVIEW: performance issue - loop over available_exprs. for e in available_exprs: if expr.same(e): self.inst_to_expr[inst] = e @@ -253,5 +270,6 @@ def get_expression( self.inst_to_expr[inst] = expr return expr + # REVIEW: dead code def get_available(self, inst: IRInstruction) -> OrderedSet[_Expression]: return self.inst_to_available.get(inst, OrderedSet()) From 5e7f60326eadb1183cc4d60ba8322e29517296ca Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 11 Nov 2024 15:34:07 +0100 Subject: [PATCH 53/59] basic refactors and bit of improvement --- vyper/venom/analysis/available_expression.py | 31 +++++++++---------- .../common_subexpression_elimination.py | 12 +++---- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index be2eb67585..535a2eacf6 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -26,20 +26,23 @@ @dataclass class _Expression: - # REVIEW: rename to root_inst - first_inst: IRInstruction + inst: IRInstruction opcode: str # REVIEW: bad type: this list only contains _Expressions, otherwise # we could not call same(self_op, other_op) in the recursion. operands: list["IROperand | _Expression"] ignore_msize: bool - # equality for lattices only based on first_inst + # equality for lattices only based on original instruction def __eq__(self, other) -> bool: if not isinstance(other, _Expression): return False - return self.first_inst == other.first_inst + return self.inst == other.inst + + def __hash__(self) -> int: + return hash(self.inst) + # Full equality for expressions based on opcode and operands # REVIEW: this is inefficient. we do not need to do the recursion if @@ -54,7 +57,7 @@ def same(self, other) -> bool: if self.opcode != other.opcode: return False - if self.first_inst == other.first_inst: + if self.inst == other.inst: return True # Early return special case for commutative instructions @@ -66,15 +69,13 @@ def same(self, other) -> bool: # General case for self_op, other_op in zip(self.operands, other.operands): + #if self_op is not other_op: #.same(other_op): + #return False if not self_op.same(other_op): return False return True - # REVIEW: move this closer in the file to __eq__ because they are - # related functions - def __hash__(self) -> int: - return hash(self.first_inst) def __repr__(self) -> str: if self.opcode == "store": @@ -98,7 +99,7 @@ def get_depth(self) -> int: @cached_property def get_reads(self) -> Effects: - tmp_reads = self.first_inst.get_read_effects() + tmp_reads = self.inst.get_read_effects() for op in self.operands: if isinstance(op, _Expression): tmp_reads = tmp_reads | op.get_reads @@ -108,7 +109,7 @@ def get_reads(self) -> Effects: @cached_property def get_writes(self) -> Effects: - tmp_reads = self.first_inst.get_write_effects() + tmp_reads = self.inst.get_write_effects() for op in self.operands: if isinstance(op, _Expression): tmp_reads = tmp_reads | op.get_writes @@ -118,11 +119,10 @@ def get_writes(self) -> Effects: @property def is_commutative(self) -> bool: - return self.first_inst.is_commutative + return self.inst.is_commutative -# REVIEW: rename to CSEAnalysis -class AvailableExpressionAnalysis(IRAnalysis): +class CSEAnalysis(IRAnalysis): inst_to_expr: dict[IRInstruction, _Expression] dfg: DFGAnalysis inst_to_available: dict[IRInstruction, OrderedSet[_Expression]] @@ -234,7 +234,7 @@ def _get_operand( # this can both create better solutions and is necessery # for correct effect handle, otherwise you could go over # effect bounderies - if inst.is_volatile: + if inst.is_volatile or inst.opcode == "phi": return op if inst in self.inst_to_expr: return self.inst_to_expr[inst] @@ -270,6 +270,5 @@ def get_expression( self.inst_to_expr[inst] = expr return expr - # REVIEW: dead code def get_available(self, inst: IRInstruction) -> OrderedSet[_Expression]: return self.inst_to_available.get(inst, OrderedSet()) diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 0897e8dc3d..10cd225810 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -1,7 +1,7 @@ from vyper.utils import OrderedSet from vyper.venom.analysis.available_expression import ( UNINTERESTING_OPCODES, - AvailableExpressionAnalysis, + CSEAnalysis, ) from vyper.venom.analysis.dfg import DFGAnalysis from vyper.venom.analysis.liveness import LivenessAnalysis @@ -13,13 +13,13 @@ class CSE(IRPass): - available_expression_analysis: AvailableExpressionAnalysis + available_expression_analysis: CSEAnalysis def run_pass(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): available_expression_analysis = self.analyses_cache.request_analysis( - AvailableExpressionAnalysis, min_depth, max_depth + CSEAnalysis, min_depth, max_depth ) - assert isinstance(available_expression_analysis, AvailableExpressionAnalysis) + assert isinstance(available_expression_analysis, CSEAnalysis) self.available_expression_analysis = available_expression_analysis while True: @@ -47,9 +47,9 @@ def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: # heuristic to not replace small expressions # basic block bounderies (it can create better codesize) if inst_expr in avail and ( - inst_expr.get_depth > 2 or inst.parent == inst_expr.first_inst.parent + inst_expr.get_depth > 2 or inst.parent == inst_expr.inst.parent ): - res[inst] = inst_expr.first_inst + res[inst] = inst_expr.inst return res From 33f3daeac0e24141ee1ba4f64f6f60d86b5d1bc3 Mon Sep 17 00:00:00 2001 From: Hodan Date: Wed, 13 Nov 2024 13:20:42 +0100 Subject: [PATCH 54/59] weird patch --- vyper/venom/effects.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/effects.py b/vyper/venom/effects.py index a668ff5439..5646b31931 100644 --- a/vyper/venom/effects.py +++ b/vyper/venom/effects.py @@ -43,7 +43,7 @@ def __iter__(self): "create2": ALL ^ (MEMORY | IMMUTABLES), "invoke": ALL, # could be smarter, look up the effects of the invoked function "log": LOG, - "dloadbytes": MEMORY, + "dloadbytes": MEMORY | IMMUTABLES, "returndatacopy": MEMORY, "calldatacopy": MEMORY, "codecopy": MEMORY, From 4e36a9c7ecc54f4e04d5aa84f3c3ed20c1d51c39 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 18 Nov 2024 09:43:11 +0100 Subject: [PATCH 55/59] different way of same fuction implementation --- .../test_common_subexpression_elimination.py | 5 - vyper/venom/__init__.py | 2 +- vyper/venom/analysis/available_expression.py | 121 ++++++++++++------ .../common_subexpression_elimination.py | 22 +++- 4 files changed, 96 insertions(+), 54 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 6cdbbc2ef6..862c0c923a 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -1,5 +1,3 @@ -import pytest - from vyper.venom.analysis.analysis import IRAnalysesCache from vyper.venom.basicblock import IRBasicBlock, IRLabel from vyper.venom.context import IRContext @@ -64,8 +62,6 @@ def test_common_subexpression_elimination_effects_1(): assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 2, "wrong number of adds" -# This is a limitation of current implementation -@pytest.mark.xfail def test_common_subexpression_elimination_effects_2(): ctx = IRContext() fn = ctx.create_function("test") @@ -140,7 +136,6 @@ def test_common_subexpression_elimination_effect_mstore(): ac = IRAnalysesCache(fn) - StoreExpansionPass(ac, fn).run_pass() CSE(ac, fn).run_pass(1, 5) assert ( diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index d8e8b73032..65d46b015a 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -67,8 +67,8 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: RemoveUnusedVariablesPass(ac, fn).run_pass() + CSE(ac, fn).run_pass(2, 100) StoreExpansionPass(ac, fn).run_pass() - CSE(ac, fn).run_pass(2, 10) RemoveUnusedVariablesPass(ac, fn).run_pass() DFTPass(ac, fn).run_pass() diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 535a2eacf6..050d9d6ad9 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -7,6 +7,7 @@ from vyper.venom.analysis.analysis import IRAnalysesCache, IRAnalysis from vyper.venom.analysis.cfg import CFGAnalysis from vyper.venom.analysis.dfg import DFGAnalysis +from vyper.venom.analysis.equivalent_vars import VarEquivalenceAnalysis from vyper.venom.basicblock import ( BB_TERMINATORS, IRBasicBlock, @@ -20,7 +21,16 @@ _MAX_DEPTH = 5 _MIN_DEPTH = 2 -UNINTERESTING_OPCODES = ["store", "param", "offset", "phi", "nop"] +UNINTERESTING_OPCODES = [ + "store", + "param", + "offset", + "phi", + "nop", + "calldatasize", + "returndatasize", + "gas", +] _NONIDEMPOTENT_INSTRUCTIONS = frozenset(["log", "call", "staticcall", "delegatecall", "invoke"]) @@ -43,42 +53,17 @@ def __eq__(self, other) -> bool: def __hash__(self) -> int: return hash(self.inst) - # Full equality for expressions based on opcode and operands # REVIEW: this is inefficient. we do not need to do the recursion if # we do a shallow comparison - we can check `self_op is other_op` for # in the loop over zip(self.operands, other.operands). because of # how expressions are computed, we are guaranteed uniqueness of expressions # in available_expressions - def same(self, other) -> bool: - if type(self) is not type(other): - return False - - if self.opcode != other.opcode: - return False - - if self.inst == other.inst: - return True - - # Early return special case for commutative instructions - if self.is_commutative: - if self.operands[0].same(other.operands[1]) and self.operands[1].same( - other.operands[0] - ): - return True - - # General case - for self_op, other_op in zip(self.operands, other.operands): - #if self_op is not other_op: #.same(other_op): - #return False - if not self_op.same(other_op): - return False - - return True - + def same(self, other, eq_vars: VarEquivalenceAnalysis) -> bool: + return same(self, other, eq_vars) def __repr__(self) -> str: - if self.opcode == "store": + if False and self.opcode == "store": assert len(self.operands) == 1, "wrong store" return repr(self.operands[0]) res = self.opcode + " [ " @@ -98,7 +83,7 @@ def get_depth(self) -> int: return max_depth + 1 @cached_property - def get_reads(self) -> Effects: + def get_reads_deep(self) -> Effects: tmp_reads = self.inst.get_read_effects() for op in self.operands: if isinstance(op, _Expression): @@ -108,7 +93,14 @@ def get_reads(self) -> Effects: return tmp_reads @cached_property - def get_writes(self) -> Effects: + def get_reads(self) -> Effects: + tmp_reads = self.inst.get_read_effects() + if self.ignore_msize: + tmp_reads &= ~Effects.MSIZE + return tmp_reads + + @cached_property + def get_writes_deep(self) -> Effects: tmp_reads = self.inst.get_write_effects() for op in self.operands: if isinstance(op, _Expression): @@ -117,16 +109,57 @@ def get_writes(self) -> Effects: tmp_reads &= ~Effects.MSIZE return tmp_reads + @cached_property + def get_writes(self) -> Effects: + tmp_reads = self.inst.get_write_effects() + if self.ignore_msize: + tmp_reads &= ~Effects.MSIZE + return tmp_reads + @property def is_commutative(self) -> bool: return self.inst.is_commutative +def same( + a: IROperand | _Expression, b: IROperand | _Expression, eq_vars: VarEquivalenceAnalysis +) -> bool: + if isinstance(a, IROperand) and isinstance(b, IROperand): + return a.value == b.value + if not isinstance(a, _Expression) or not isinstance(b, _Expression): + return False + + if a.opcode != b.opcode: + return False + + if a.inst == b.inst: + return True + + # Early return special case for commutative instructions + if a.is_commutative: + if same(a.operands[0], b.operands[1], eq_vars) and same( + a.operands[1], b.operands[0], eq_vars + ): + return True + + # General case + for self_op, other_op in zip(a.operands, b.operands): + if ( + self_op is not other_op + and not eq_vars.equivalent(self_op, other_op) + and self_op != other_op + ): + return False + + return True + + class CSEAnalysis(IRAnalysis): inst_to_expr: dict[IRInstruction, _Expression] dfg: DFGAnalysis inst_to_available: dict[IRInstruction, OrderedSet[_Expression]] bb_outs: dict[IRBasicBlock, OrderedSet[_Expression]] + eq_vars: VarEquivalenceAnalysis # the size of the expressions # that are considered in the analysis @@ -146,6 +179,7 @@ def __init__( dfg = self.analyses_cache.request_analysis(DFGAnalysis) assert isinstance(dfg, DFGAnalysis) self.dfg = dfg + self.eq_vars = self.analyses_cache.request_analysis(VarEquivalenceAnalysis) # type: ignore self.min_depth = min_depth self.max_depth = max_depth @@ -191,7 +225,8 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: # bb_lat = self.lattice.data[bb] change = False for inst in bb.instructions: - if inst.opcode in UNINTERESTING_OPCODES or inst.opcode in BB_TERMINATORS: + # if inst.opcode in UNINTERESTING_OPCODES or inst.opcode in BB_TERMINATORS: + if inst.opcode in BB_TERMINATORS: continue # REVIEW: why replace inst_to_available if they are not equal? @@ -208,13 +243,7 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: if write_effects_expr & write_effects != EMPTY: available_expr.remove(expr) - # REVIEW: we should not care about depth if `same()` is not - # implemented recursively. - if ( - inst_expr.get_depth in range(self.min_depth, self.max_depth + 1) - and inst.opcode not in _NONIDEMPOTENT_INSTRUCTIONS - and write_effects & inst_expr.get_reads == EMPTY - ): + if inst_expr.get_writes_deep & inst_expr.get_reads_deep == EMPTY: available_expr.add(inst_expr) if bb not in self.bb_outs or available_expr != self.bb_outs[bb]: @@ -228,7 +257,7 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: def _get_operand( self, op: IROperand, available_exprs: OrderedSet[_Expression], depth: int ) -> IROperand | _Expression: - if depth > 0 and isinstance(op, IRVariable): + if isinstance(op, IRVariable): inst = self.dfg.get_producing_instruction(op) assert inst is not None # this can both create better solutions and is necessery @@ -236,6 +265,8 @@ def _get_operand( # effect bounderies if inst.is_volatile or inst.opcode == "phi": return op + if inst.opcode == "store": + return self._get_operand(inst.operands[0], available_exprs, depth - 1) if inst in self.inst_to_expr: return self.inst_to_expr[inst] return self.get_expression(inst, available_exprs, depth - 1) @@ -261,9 +292,17 @@ def get_expression( if inst in self.inst_to_expr and self.inst_to_expr[inst] in available_exprs: return self.inst_to_expr[inst] + def cond() -> bool: + return False + return ( + inst.opcode == "add" + and isinstance(inst.operands[0], IRVariable) + and inst.operands[0].value == "%13:3" + ) + # REVIEW: performance issue - loop over available_exprs. for e in available_exprs: - if expr.same(e): + if expr.same(e, self.eq_vars): self.inst_to_expr[inst] = e return e diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 10cd225810..2778a1114e 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -1,5 +1,6 @@ from vyper.utils import OrderedSet from vyper.venom.analysis.available_expression import ( + _NONIDEMPOTENT_INSTRUCTIONS, UNINTERESTING_OPCODES, CSEAnalysis, ) @@ -13,14 +14,14 @@ class CSE(IRPass): - available_expression_analysis: CSEAnalysis + expression_analysis: CSEAnalysis def run_pass(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): available_expression_analysis = self.analyses_cache.request_analysis( CSEAnalysis, min_depth, max_depth ) assert isinstance(available_expression_analysis, CSEAnalysis) - self.available_expression_analysis = available_expression_analysis + self.expression_analysis = available_expression_analysis while True: replace_dict = self._find_replaceble() @@ -30,24 +31,31 @@ def run_pass(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): self.analyses_cache.invalidate_analysis(DFGAnalysis) self.analyses_cache.invalidate_analysis(LivenessAnalysis) # should be ok to be reevaluted - self.available_expression_analysis.analyze(min_depth, max_depth) + # self.available_expression_analysis.analyze(min_depth, max_depth) + self.expression_analysis = self.analyses_cache.force_analysis( + CSEAnalysis, min_depth, max_depth + ) # type: ignore # return instruction and to which instruction it could # replaced by def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: res: dict[IRInstruction, IRInstruction] = dict() + for bb in self.function.get_basic_blocks(): for inst in bb.instructions: # skip instruction that for sure # wont be substituted - if inst in UNINTERESTING_OPCODES: + if ( + inst.opcode in UNINTERESTING_OPCODES + or inst.opcode in _NONIDEMPOTENT_INSTRUCTIONS + ): continue - inst_expr = self.available_expression_analysis.get_expression(inst) - avail = self.available_expression_analysis.get_available(inst) + inst_expr = self.expression_analysis.get_expression(inst) + avail = self.expression_analysis.get_available(inst) # heuristic to not replace small expressions # basic block bounderies (it can create better codesize) if inst_expr in avail and ( - inst_expr.get_depth > 2 or inst.parent == inst_expr.inst.parent + inst_expr.get_depth > 1 or inst.parent == inst_expr.inst.parent ): res[inst] = inst_expr.inst From dc07dff7984acf2ddfdc264b2213209f8d7f19eb Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 18 Nov 2024 11:28:13 +0100 Subject: [PATCH 56/59] small cleanup and more unintresting instruction --- vyper/venom/analysis/available_expression.py | 62 +++++++++++-------- vyper/venom/basicblock.py | 5 -- .../common_subexpression_elimination.py | 4 +- 3 files changed, 37 insertions(+), 34 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index 050d9d6ad9..d170098156 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -21,25 +21,44 @@ _MAX_DEPTH = 5 _MIN_DEPTH = 2 -UNINTERESTING_OPCODES = [ - "store", - "param", - "offset", - "phi", - "nop", - "calldatasize", - "returndatasize", - "gas", -] -_NONIDEMPOTENT_INSTRUCTIONS = frozenset(["log", "call", "staticcall", "delegatecall", "invoke"]) +UNINTERESTING_OPCODES = frozenset( + [ + "store", + "param", + "offset", + "phi", + "nop", + "calldatasize", + "returndatasize", + "gas", + "gaslimit", + "gasprice", + "gaslimit", + "address", + "origin", + "codesize", + "caller", + "callvalue", + "coinbase", + "timestamp", + "number", + "prevrandao", + "chainid", + "basefee", + "blobbasefee", + "pc", + "msize", + ] +) +NONIDEMPOTENT_INSTRUCTIONS = frozenset(["log", "call", "staticcall", "delegatecall", "invoke"]) @dataclass class _Expression: inst: IRInstruction opcode: str - # REVIEW: bad type: this list only contains _Expressions, otherwise - # we could not call same(self_op, other_op) in the recursion. + # the child is either expression of operand since + # there are possibilities for cycles operands: list["IROperand | _Expression"] ignore_msize: bool @@ -54,16 +73,11 @@ def __hash__(self) -> int: return hash(self.inst) # Full equality for expressions based on opcode and operands - # REVIEW: this is inefficient. we do not need to do the recursion if - # we do a shallow comparison - we can check `self_op is other_op` for - # in the loop over zip(self.operands, other.operands). because of - # how expressions are computed, we are guaranteed uniqueness of expressions - # in available_expressions def same(self, other, eq_vars: VarEquivalenceAnalysis) -> bool: return same(self, other, eq_vars) def __repr__(self) -> str: - if False and self.opcode == "store": + if self.opcode == "store": assert len(self.operands) == 1, "wrong store" return repr(self.operands[0]) res = self.opcode + " [ " @@ -263,6 +277,8 @@ def _get_operand( # this can both create better solutions and is necessery # for correct effect handle, otherwise you could go over # effect bounderies + # the phi condition is here because it is only way to + # create call loop if inst.is_volatile or inst.opcode == "phi": return op if inst.opcode == "store": @@ -292,14 +308,6 @@ def get_expression( if inst in self.inst_to_expr and self.inst_to_expr[inst] in available_exprs: return self.inst_to_expr[inst] - def cond() -> bool: - return False - return ( - inst.opcode == "add" - and isinstance(inst.operands[0], IRVariable) - and inst.operands[0].value == "%13:3" - ) - # REVIEW: performance issue - loop over available_exprs. for e in available_exprs: if expr.same(e, self.eq_vars): diff --git a/vyper/venom/basicblock.py b/vyper/venom/basicblock.py index 949e116510..c0abcefcb0 100644 --- a/vyper/venom/basicblock.py +++ b/vyper/venom/basicblock.py @@ -126,11 +126,6 @@ def __eq__(self, other) -> bool: return False return self.value == other.value - def same(self, other) -> bool: - if not isinstance(other, type(self)): - return False - return self.value == other.value - def __repr__(self) -> str: return str(self.value) diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 2778a1114e..34ff101eb2 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -1,6 +1,6 @@ from vyper.utils import OrderedSet from vyper.venom.analysis.available_expression import ( - _NONIDEMPOTENT_INSTRUCTIONS, + NONIDEMPOTENT_INSTRUCTIONS, UNINTERESTING_OPCODES, CSEAnalysis, ) @@ -47,7 +47,7 @@ def _find_replaceble(self) -> dict[IRInstruction, IRInstruction]: # wont be substituted if ( inst.opcode in UNINTERESTING_OPCODES - or inst.opcode in _NONIDEMPOTENT_INSTRUCTIONS + or inst.opcode in NONIDEMPOTENT_INSTRUCTIONS ): continue inst_expr = self.expression_analysis.get_expression(inst) From 4d571711cfcba03c5250e8ea50a78fc4456620d7 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 18 Nov 2024 13:07:04 +0100 Subject: [PATCH 57/59] removed depth --- .../test_common_subexpression_elimination.py | 10 +-- vyper/venom/__init__.py | 2 +- vyper/venom/analysis/available_expression.py | 74 +++++-------------- .../common_subexpression_elimination.py | 43 +++++++++-- 4 files changed, 59 insertions(+), 70 deletions(-) diff --git a/tests/unit/compiler/venom/test_common_subexpression_elimination.py b/tests/unit/compiler/venom/test_common_subexpression_elimination.py index 862c0c923a..85f3fd7f07 100644 --- a/tests/unit/compiler/venom/test_common_subexpression_elimination.py +++ b/tests/unit/compiler/venom/test_common_subexpression_elimination.py @@ -18,7 +18,7 @@ def test_common_subexpression_elimination(): ac = IRAnalysesCache(fn) - CSE(ac, fn).run_pass(1, 5) + CSE(ac, fn).run_pass() assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 1, "wrong number of adds" assert sum(1 for inst in bb.instructions if inst.opcode == "mul") == 1, "wrong number of muls" @@ -37,7 +37,7 @@ def test_common_subexpression_elimination_commutative(): ac = IRAnalysesCache(fn) - CSE(ac, fn).run_pass(1, 5) + CSE(ac, fn).run_pass() assert sum(1 for inst in bb.instructions if inst.opcode == "add") == 1, "wrong number of adds" assert sum(1 for inst in bb.instructions if inst.opcode == "mul") == 1, "wrong number of muls" @@ -136,7 +136,7 @@ def test_common_subexpression_elimination_effect_mstore(): ac = IRAnalysesCache(fn) - CSE(ac, fn).run_pass(1, 5) + CSE(ac, fn).run_pass() assert ( sum(1 for inst in bb.instructions if inst.opcode == "mstore") == 1 @@ -164,7 +164,7 @@ def test_common_subexpression_elimination_effect_mstore_with_msize(): ac = IRAnalysesCache(fn) StoreExpansionPass(ac, fn).run_pass() - CSE(ac, fn).run_pass(1, 5) + CSE(ac, fn).run_pass() assert ( sum(1 for inst in bb.instructions if inst.opcode == "mstore") == 2 @@ -206,7 +206,7 @@ def do_same(bb: IRBasicBlock, rand: int): ac = IRAnalysesCache(fn) StoreExpansionPass(ac, fn).run_pass() - CSE(ac, fn).run_pass(1, 5) + CSE(ac, fn).run_pass() assert sum(1 for inst in br1.instructions if inst.opcode == "add") == 1, "wrong number of adds" assert sum(1 for inst in br2.instructions if inst.opcode == "add") == 1, "wrong number of adds" diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 65d46b015a..626cdec90b 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -67,7 +67,7 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: RemoveUnusedVariablesPass(ac, fn).run_pass() - CSE(ac, fn).run_pass(2, 100) + CSE(ac, fn).run_pass() StoreExpansionPass(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() DFTPass(ac, fn).run_pass() diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index d170098156..b55eaa3288 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -18,39 +18,6 @@ from vyper.venom.context import IRFunction from vyper.venom.effects import EMPTY, Effects -_MAX_DEPTH = 5 -_MIN_DEPTH = 2 - -UNINTERESTING_OPCODES = frozenset( - [ - "store", - "param", - "offset", - "phi", - "nop", - "calldatasize", - "returndatasize", - "gas", - "gaslimit", - "gasprice", - "gaslimit", - "address", - "origin", - "codesize", - "caller", - "callvalue", - "coinbase", - "timestamp", - "number", - "prevrandao", - "chainid", - "basefee", - "blobbasefee", - "pc", - "msize", - ] -) -NONIDEMPOTENT_INSTRUCTIONS = frozenset(["log", "call", "staticcall", "delegatecall", "invoke"]) @dataclass @@ -143,12 +110,12 @@ def same( if not isinstance(a, _Expression) or not isinstance(b, _Expression): return False - if a.opcode != b.opcode: - return False - if a.inst == b.inst: return True + if a.opcode != b.opcode: + return False + # Early return special case for commutative instructions if a.is_commutative: if same(a.operands[0], b.operands[1], eq_vars) and same( @@ -175,18 +142,12 @@ class CSEAnalysis(IRAnalysis): bb_outs: dict[IRBasicBlock, OrderedSet[_Expression]] eq_vars: VarEquivalenceAnalysis - # the size of the expressions - # that are considered in the analysis - min_depth: int - max_depth: int ignore_msize: bool def __init__( self, analyses_cache: IRAnalysesCache, function: IRFunction, - min_depth: int = _MIN_DEPTH, - max_depth: int = _MAX_DEPTH, ): super().__init__(analyses_cache, function) self.analyses_cache.request_analysis(CFGAnalysis) @@ -195,19 +156,13 @@ def __init__( self.dfg = dfg self.eq_vars = self.analyses_cache.request_analysis(VarEquivalenceAnalysis) # type: ignore - self.min_depth = min_depth - self.max_depth = max_depth - self.inst_to_expr = dict() self.inst_to_available = dict() self.bb_outs = dict() self.ignore_msize = not self._contains_msize() - def analyze(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): - self.min_depth = min_depth - self.max_depth = max_depth - + def analyze(self): worklist: OrderedSet = OrderedSet() worklist.add(self.function.entry) while len(worklist) > 0: @@ -269,7 +224,7 @@ def _handle_bb(self, bb: IRBasicBlock) -> bool: return change def _get_operand( - self, op: IROperand, available_exprs: OrderedSet[_Expression], depth: int + self, op: IROperand, available_exprs: OrderedSet[_Expression] ) -> IROperand | _Expression: if isinstance(op, IRVariable): inst = self.dfg.get_producing_instruction(op) @@ -282,33 +237,38 @@ def _get_operand( if inst.is_volatile or inst.opcode == "phi": return op if inst.opcode == "store": - return self._get_operand(inst.operands[0], available_exprs, depth - 1) + return self._get_operand(inst.operands[0], available_exprs) if inst in self.inst_to_expr: return self.inst_to_expr[inst] - return self.get_expression(inst, available_exprs, depth - 1) + return self.get_expression(inst, available_exprs) return op def _get_operands( - self, inst: IRInstruction, available_exprs: OrderedSet[_Expression], depth: int + self, inst: IRInstruction, available_exprs: OrderedSet[_Expression] ) -> list[IROperand | _Expression]: - return [self._get_operand(op, available_exprs, depth) for op in inst.operands] + return [self._get_operand(op, available_exprs) for op in inst.operands] def get_expression( self, inst: IRInstruction, available_exprs: OrderedSet[_Expression] | None = None, - depth: int | None = None, ) -> _Expression: available_exprs = available_exprs or self.inst_to_available.get(inst, OrderedSet()) assert available_exprs is not None - depth = self.max_depth if depth is None else depth - operands: list[IROperand | _Expression] = self._get_operands(inst, available_exprs, depth) + operands: list[IROperand | _Expression] = self._get_operands(inst, available_exprs) expr = _Expression(inst, inst.opcode, operands, self.ignore_msize) if inst in self.inst_to_expr and self.inst_to_expr[inst] in available_exprs: return self.inst_to_expr[inst] # REVIEW: performance issue - loop over available_exprs. + #e = next((e for e in available_exprs if expr.same(e, self.eq_vars)), expr) + #self.inst_to_expr[inst] = e + #return e + #if any(expr.same(e, self.eq_vars) for e in available_exprs): + #e = next(e for e in available_exprs if expr.same(e, self.eq_vars)) + #self.inst_to_expr[inst] = e + #return e for e in available_exprs: if expr.same(e, self.eq_vars): self.inst_to_expr[inst] = e diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index 34ff101eb2..e845cb4675 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -1,7 +1,5 @@ from vyper.utils import OrderedSet from vyper.venom.analysis.available_expression import ( - NONIDEMPOTENT_INSTRUCTIONS, - UNINTERESTING_OPCODES, CSEAnalysis, ) from vyper.venom.analysis.dfg import DFGAnalysis @@ -9,16 +7,47 @@ from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRVariable from vyper.venom.passes.base_pass import IRPass -_MAX_DEPTH = 5 -_MIN_DEPTH = 2 +# instruction that are not usefull to be +# substituted +UNINTERESTING_OPCODES = frozenset( + [ + "store", + "param", + "offset", + "phi", + "nop", + "calldatasize", + "returndatasize", + "gas", + "gaslimit", + "gasprice", + "gaslimit", + "address", + "origin", + "codesize", + "caller", + "callvalue", + "coinbase", + "timestamp", + "number", + "prevrandao", + "chainid", + "basefee", + "blobbasefee", + "pc", + "msize", + ] +) +# intruction that cannot be substituted (without further analysis) +NONIDEMPOTENT_INSTRUCTIONS = frozenset(["log", "call", "staticcall", "delegatecall", "invoke"]) class CSE(IRPass): expression_analysis: CSEAnalysis - def run_pass(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): + def run_pass(self): available_expression_analysis = self.analyses_cache.request_analysis( - CSEAnalysis, min_depth, max_depth + CSEAnalysis ) assert isinstance(available_expression_analysis, CSEAnalysis) self.expression_analysis = available_expression_analysis @@ -33,7 +62,7 @@ def run_pass(self, min_depth: int = _MIN_DEPTH, max_depth: int = _MAX_DEPTH): # should be ok to be reevaluted # self.available_expression_analysis.analyze(min_depth, max_depth) self.expression_analysis = self.analyses_cache.force_analysis( - CSEAnalysis, min_depth, max_depth + CSEAnalysis ) # type: ignore # return instruction and to which instruction it could From 1a9c9abc41a5d1d18332a8d4ebf7585fcc64e197 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 18 Nov 2024 13:08:45 +0100 Subject: [PATCH 58/59] lint --- vyper/venom/analysis/available_expression.py | 25 +++++++------------ .../common_subexpression_elimination.py | 10 +++----- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index b55eaa3288..d09237b7a8 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -19,7 +19,6 @@ from vyper.venom.effects import EMPTY, Effects - @dataclass class _Expression: inst: IRInstruction @@ -144,11 +143,7 @@ class CSEAnalysis(IRAnalysis): ignore_msize: bool - def __init__( - self, - analyses_cache: IRAnalysesCache, - function: IRFunction, - ): + def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): super().__init__(analyses_cache, function) self.analyses_cache.request_analysis(CFGAnalysis) dfg = self.analyses_cache.request_analysis(DFGAnalysis) @@ -249,9 +244,7 @@ def _get_operands( return [self._get_operand(op, available_exprs) for op in inst.operands] def get_expression( - self, - inst: IRInstruction, - available_exprs: OrderedSet[_Expression] | None = None, + self, inst: IRInstruction, available_exprs: OrderedSet[_Expression] | None = None ) -> _Expression: available_exprs = available_exprs or self.inst_to_available.get(inst, OrderedSet()) assert available_exprs is not None @@ -262,13 +255,13 @@ def get_expression( return self.inst_to_expr[inst] # REVIEW: performance issue - loop over available_exprs. - #e = next((e for e in available_exprs if expr.same(e, self.eq_vars)), expr) - #self.inst_to_expr[inst] = e - #return e - #if any(expr.same(e, self.eq_vars) for e in available_exprs): - #e = next(e for e in available_exprs if expr.same(e, self.eq_vars)) - #self.inst_to_expr[inst] = e - #return e + # e = next((e for e in available_exprs if expr.same(e, self.eq_vars)), expr) + # self.inst_to_expr[inst] = e + # return e + # if any(expr.same(e, self.eq_vars) for e in available_exprs): + # e = next(e for e in available_exprs if expr.same(e, self.eq_vars)) + # self.inst_to_expr[inst] = e + # return e for e in available_exprs: if expr.same(e, self.eq_vars): self.inst_to_expr[inst] = e diff --git a/vyper/venom/passes/common_subexpression_elimination.py b/vyper/venom/passes/common_subexpression_elimination.py index e845cb4675..a6384d273e 100644 --- a/vyper/venom/passes/common_subexpression_elimination.py +++ b/vyper/venom/passes/common_subexpression_elimination.py @@ -1,13 +1,10 @@ from vyper.utils import OrderedSet -from vyper.venom.analysis.available_expression import ( - CSEAnalysis, -) +from vyper.venom.analysis.available_expression import CSEAnalysis from vyper.venom.analysis.dfg import DFGAnalysis from vyper.venom.analysis.liveness import LivenessAnalysis from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRVariable from vyper.venom.passes.base_pass import IRPass - # instruction that are not usefull to be # substituted UNINTERESTING_OPCODES = frozenset( @@ -42,13 +39,12 @@ # intruction that cannot be substituted (without further analysis) NONIDEMPOTENT_INSTRUCTIONS = frozenset(["log", "call", "staticcall", "delegatecall", "invoke"]) + class CSE(IRPass): expression_analysis: CSEAnalysis def run_pass(self): - available_expression_analysis = self.analyses_cache.request_analysis( - CSEAnalysis - ) + available_expression_analysis = self.analyses_cache.request_analysis(CSEAnalysis) assert isinstance(available_expression_analysis, CSEAnalysis) self.expression_analysis = available_expression_analysis From 639d70b5911f864af8370447b8790b1dd2beea69 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 18 Nov 2024 13:17:51 +0100 Subject: [PATCH 59/59] removed forgoten comment --- vyper/venom/analysis/available_expression.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/vyper/venom/analysis/available_expression.py b/vyper/venom/analysis/available_expression.py index d09237b7a8..6d85ea7faa 100644 --- a/vyper/venom/analysis/available_expression.py +++ b/vyper/venom/analysis/available_expression.py @@ -255,13 +255,6 @@ def get_expression( return self.inst_to_expr[inst] # REVIEW: performance issue - loop over available_exprs. - # e = next((e for e in available_exprs if expr.same(e, self.eq_vars)), expr) - # self.inst_to_expr[inst] = e - # return e - # if any(expr.same(e, self.eq_vars) for e in available_exprs): - # e = next(e for e in available_exprs if expr.same(e, self.eq_vars)) - # self.inst_to_expr[inst] = e - # return e for e in available_exprs: if expr.same(e, self.eq_vars): self.inst_to_expr[inst] = e