From 957c8edd834e4991d7470082b179e4a1a1ab7e4d Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 25 Sep 2024 11:27:07 -0400 Subject: [PATCH 1/5] perf[venom]: improve OrderedSet operations (#4246) improve time spent in venom by 25% dict views support set operations, which allows us to loop, using `&=`. since our intersections are typically not between a large number of OrderedSets, this results in faster execution time. references: - https://docs.python.org/3/library/stdtypes.html#dict-views --- vyper/utils.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/vyper/utils.py b/vyper/utils.py index f4f14a346e..3f19a9d15c 100644 --- a/vyper/utils.py +++ b/vyper/utils.py @@ -25,9 +25,10 @@ class OrderedSet(Generic[_T]): """ def __init__(self, iterable=None): - self._data = dict() - if iterable is not None: - self.update(iterable) + if iterable is None: + self._data = dict() + else: + self._data = dict.fromkeys(iterable) def __repr__(self): keys = ", ".join(repr(k) for k in self) @@ -57,6 +58,7 @@ def pop(self): def add(self, item: _T) -> None: self._data[item] = None + # NOTE to refactor: duplicate of self.update() def addmany(self, iterable): for item in iterable: self._data[item] = None @@ -109,11 +111,11 @@ def intersection(cls, *sets): if len(sets) == 0: raise ValueError("undefined: intersection of no sets") - ret = sets[0].copy() - for e in sets[0]: - if any(e not in s for s in sets[1:]): - ret.remove(e) - return ret + tmp = sets[0]._data.keys() + for s in sets[1:]: + tmp &= s._data.keys() + + return cls(tmp) class StringEnum(enum.Enum): From d60d31f3375db5b1f867c209316d9c29aca5d8b3 Mon Sep 17 00:00:00 2001 From: trocher Date: Thu, 26 Sep 2024 02:52:57 +0200 Subject: [PATCH 2/5] fix[lang]: fix `==` and `!=` bytesM folding (#4254) `bytesM` literals are not case-sensitive (they represent the same value no matter if the literal is lower- or upper-case), but the folding operation was case sensitive. --------- Co-authored-by: Charles Cooper --- tests/unit/ast/nodes/test_fold_compare.py | 17 +++++++++++++++++ vyper/semantics/analysis/constant_folding.py | 7 +++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/unit/ast/nodes/test_fold_compare.py b/tests/unit/ast/nodes/test_fold_compare.py index aab8ac0b2d..fd9f65a7d3 100644 --- a/tests/unit/ast/nodes/test_fold_compare.py +++ b/tests/unit/ast/nodes/test_fold_compare.py @@ -110,3 +110,20 @@ def test_compare_type_mismatch(op): old_node = vyper_ast.body[0].value with pytest.raises(UnfoldableNode): old_node.get_folded_value() + + +@pytest.mark.parametrize("op", ["==", "!="]) +def test_compare_eq_bytes(get_contract, op): + left, right = "0xA1AAB33F", "0xa1aab33f" + source = f""" +@external +def foo(a: bytes4, b: bytes4) -> bool: + return a {op} b + """ + contract = get_contract(source) + + vyper_ast = parse_and_fold(f"{left} {op} {right}") + old_node = vyper_ast.body[0].value + new_node = old_node.get_folded_value() + + assert contract.foo(left, right) == new_node.value diff --git a/vyper/semantics/analysis/constant_folding.py b/vyper/semantics/analysis/constant_folding.py index 6e4166dc52..98cab0f8cb 100644 --- a/vyper/semantics/analysis/constant_folding.py +++ b/vyper/semantics/analysis/constant_folding.py @@ -180,8 +180,11 @@ def visit_Compare(self, node): raise UnfoldableNode( f"Invalid literal types for {node.op.description} comparison", node ) - - value = node.op._op(left.value, right.value) + lvalue, rvalue = left.value, right.value + if isinstance(left, vy_ast.Hex): + # Hex values are str, convert to be case-unsensitive. + lvalue, rvalue = lvalue.lower(), rvalue.lower() + value = node.op._op(lvalue, rvalue) return vy_ast.NameConstant.from_node(node, value=value) def visit_List(self, node) -> vy_ast.ExprNode: From e2f60019029139f2a69523a97586c6e404e00a37 Mon Sep 17 00:00:00 2001 From: HodanPlodky <36966616+HodanPlodky@users.noreply.github.com> Date: Fri, 27 Sep 2024 01:21:57 +0000 Subject: [PATCH 3/5] fix[venom]: fix invalid `phi`s after SCCP (#4181) This commit reduces `phi` nodes which, after SCCP, refer to a basic block which is unreachable. This could happen when the SCCP would replace a `jnz` by a `jmp` instruction, resulting in a `phi` instruction which refers to a non-existent block, or a `phi` instruction in a basic block which only has one predecessor. This commit reduces such `phi` instructions into `store` instructions. --------- Co-authored-by: Harry Kalogirou Co-authored-by: Charles Cooper --- tests/unit/compiler/venom/test_sccp.py | 31 ++++++++++++ .../unit/compiler/venom/test_simplify_cfg.py | 49 +++++++++++++++++++ vyper/venom/passes/sccp/sccp.py | 42 +++++++++++++++- vyper/venom/passes/simplify_cfg.py | 16 +++--- 4 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 tests/unit/compiler/venom/test_simplify_cfg.py diff --git a/tests/unit/compiler/venom/test_sccp.py b/tests/unit/compiler/venom/test_sccp.py index e65839136e..478acc1079 100644 --- a/tests/unit/compiler/venom/test_sccp.py +++ b/tests/unit/compiler/venom/test_sccp.py @@ -211,3 +211,34 @@ def test_cont_phi_const_case(): assert sccp.lattice[IRVariable("%5", version=1)].value == 106 assert sccp.lattice[IRVariable("%5", version=2)].value == 97 assert sccp.lattice[IRVariable("%5")].value == 2 + + +def test_phi_reduction_after_unreachable_block(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + + br1 = IRBasicBlock(IRLabel("then"), fn) + fn.append_basic_block(br1) + join = IRBasicBlock(IRLabel("join"), fn) + fn.append_basic_block(join) + + op = bb.append_instruction("store", 1) + true = IRLiteral(1) + bb.append_instruction("jnz", true, br1.label, join.label) + + op1 = br1.append_instruction("store", 2) + + br1.append_instruction("jmp", join.label) + + join.append_instruction("phi", bb.label, op, br1.label, op1) + join.append_instruction("stop") + + ac = IRAnalysesCache(fn) + SCCP(ac, fn).run_pass() + + assert join.instructions[0].opcode == "store", join.instructions[0] + assert join.instructions[0].operands == [op1] + + assert join.instructions[1].opcode == "stop" diff --git a/tests/unit/compiler/venom/test_simplify_cfg.py b/tests/unit/compiler/venom/test_simplify_cfg.py new file mode 100644 index 0000000000..c4bdbb263b --- /dev/null +++ b/tests/unit/compiler/venom/test_simplify_cfg.py @@ -0,0 +1,49 @@ +from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRLiteral +from vyper.venom.context import IRContext +from vyper.venom.passes.sccp import SCCP +from vyper.venom.passes.simplify_cfg import SimplifyCFGPass + + +def test_phi_reduction_after_block_pruning(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + + br1 = IRBasicBlock(IRLabel("then"), fn) + fn.append_basic_block(br1) + br2 = IRBasicBlock(IRLabel("else"), fn) + fn.append_basic_block(br2) + + join = IRBasicBlock(IRLabel("join"), fn) + fn.append_basic_block(join) + + true = IRLiteral(1) + bb.append_instruction("jnz", true, br1.label, br2.label) + + op1 = br1.append_instruction("store", 1) + op2 = br2.append_instruction("store", 2) + + br1.append_instruction("jmp", join.label) + br2.append_instruction("jmp", join.label) + + join.append_instruction("phi", br1.label, op1, br2.label, op2) + join.append_instruction("stop") + + ac = IRAnalysesCache(fn) + SCCP(ac, fn).run_pass() + SimplifyCFGPass(ac, fn).run_pass() + + bbs = list(fn.get_basic_blocks()) + + assert len(bbs) == 1 + final_bb = bbs[0] + + inst0, inst1, inst2 = final_bb.instructions + + assert inst0.opcode == "store" + assert inst0.operands == [IRLiteral(1)] + assert inst1.opcode == "store" + assert inst1.operands == [inst0.output] + assert inst2.opcode == "stop" diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index 164d8e241d..cfac6794f8 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -56,14 +56,18 @@ class SCCP(IRPass): uses: dict[IRVariable, OrderedSet[IRInstruction]] lattice: Lattice work_list: list[WorkListItem] - cfg_dirty: bool cfg_in_exec: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] + cfg_dirty: bool + # list of basic blocks whose cfg_in was modified + cfg_in_modified: OrderedSet[IRBasicBlock] + def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): super().__init__(analyses_cache, function) self.lattice = {} self.work_list: list[WorkListItem] = [] self.cfg_dirty = False + self.cfg_in_modified = OrderedSet() def run_pass(self): self.fn = self.function @@ -74,7 +78,9 @@ def run_pass(self): # self._propagate_variables() - self.analyses_cache.invalidate_analysis(CFGAnalysis) + if self.cfg_dirty: + self.analyses_cache.force_analysis(CFGAnalysis) + self._fix_phi_nodes() def _calculate_sccp(self, entry: IRBasicBlock): """ @@ -304,7 +310,11 @@ def _replace_constants(self, inst: IRInstruction): target = inst.operands[1] inst.opcode = "jmp" inst.operands = [target] + self.cfg_dirty = True + for bb in inst.parent.cfg_out: + if bb.label == target: + self.cfg_in_modified.add(bb) elif inst.opcode in ("assert", "assert_unreachable"): lat = self._eval_from_lattice(inst.operands[0]) @@ -329,6 +339,34 @@ def _replace_constants(self, inst: IRInstruction): if isinstance(lat, IRLiteral): inst.operands[i] = lat + def _fix_phi_nodes(self): + # fix basic blocks whose cfg in was changed + # maybe this should really be done in _visit_phi + needs_sort = False + + for bb in self.fn.get_basic_blocks(): + cfg_in_labels = OrderedSet(in_bb.label for in_bb in bb.cfg_in) + + for inst in bb.instructions: + if inst.opcode != "phi": + break + needs_sort |= self._fix_phi_inst(inst, cfg_in_labels) + + # move phi instructions to the top of the block + if needs_sort: + bb.instructions.sort(key=lambda inst: inst.opcode != "phi") + + def _fix_phi_inst(self, inst: IRInstruction, cfg_in_labels: OrderedSet): + operands = [op for label, op in inst.phi_operands if label in cfg_in_labels] + + if len(operands) != 1: + return False + + assert inst.output is not None + inst.opcode = "store" + inst.operands = operands + return True + def _meet(x: LatticeItem, y: LatticeItem) -> LatticeItem: if x == LatticeEnum.TOP: diff --git a/vyper/venom/passes/simplify_cfg.py b/vyper/venom/passes/simplify_cfg.py index 08582fee96..1409f43947 100644 --- a/vyper/venom/passes/simplify_cfg.py +++ b/vyper/venom/passes/simplify_cfg.py @@ -9,23 +9,21 @@ class SimplifyCFGPass(IRPass): visited: OrderedSet def _merge_blocks(self, a: IRBasicBlock, b: IRBasicBlock): - a.instructions.pop() + a.instructions.pop() # pop terminating instruction for inst in b.instructions: - assert inst.opcode != "phi", "Not implemented yet" - if inst.opcode == "phi": - a.instructions.insert(0, inst) - else: - inst.parent = a - a.instructions.append(inst) + assert inst.opcode != "phi", f"Instruction should never be phi {b}" + inst.parent = a + a.instructions.append(inst) # Update CFG a.cfg_out = b.cfg_out - if len(b.cfg_out) > 0: - next_bb = b.cfg_out.first() + + for next_bb in a.cfg_out: next_bb.remove_cfg_in(b) next_bb.add_cfg_in(a) for inst in next_bb.instructions: + # assume phi instructions are at beginning of bb if inst.opcode != "phi": break inst.operands[inst.operands.index(b.label)] = a.label From b83bc98d16440c430a2d1ccd04b34054e3bfe9aa Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 28 Sep 2024 11:02:51 -0400 Subject: [PATCH 4/5] fix[venom]: clean up sccp pass (#4261) small cleanup tasks for sccp pass. remove dead variables and stray comments. --- vyper/venom/passes/sccp/sccp.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index cfac6794f8..013583ec63 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -59,15 +59,12 @@ class SCCP(IRPass): cfg_in_exec: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] cfg_dirty: bool - # list of basic blocks whose cfg_in was modified - cfg_in_modified: OrderedSet[IRBasicBlock] def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): super().__init__(analyses_cache, function) self.lattice = {} self.work_list: list[WorkListItem] = [] self.cfg_dirty = False - self.cfg_in_modified = OrderedSet() def run_pass(self): self.fn = self.function @@ -76,8 +73,6 @@ def run_pass(self): self._calculate_sccp(self.fn.entry) self._propagate_constants() - # self._propagate_variables() - if self.cfg_dirty: self.analyses_cache.force_analysis(CFGAnalysis) self._fix_phi_nodes() @@ -312,9 +307,6 @@ def _replace_constants(self, inst: IRInstruction): inst.operands = [target] self.cfg_dirty = True - for bb in inst.parent.cfg_out: - if bb.label == target: - self.cfg_in_modified.add(bb) elif inst.opcode in ("assert", "assert_unreachable"): lat = self._eval_from_lattice(inst.operands[0]) From e21f3e8b1f7d5a5b5b40fb73ab4c6a2946e878ef Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 28 Sep 2024 11:08:03 -0400 Subject: [PATCH 5/5] refactor[venom]: remove `dup_requirements` analysis (#4262) this commit updates `next_liveness` to reflect `out_vars` when the instruction is the last instruction in the basic block. after this change, membership in `dup_requirements` is the same as membership in `next_liveness`, so we can remove the `dup_requirements` analysis. --- vyper/venom/analysis/dup_requirements.py | 15 --------------- vyper/venom/basicblock.py | 2 -- vyper/venom/venom_to_assembly.py | 24 ++++++++++++++---------- 3 files changed, 14 insertions(+), 27 deletions(-) delete mode 100644 vyper/venom/analysis/dup_requirements.py diff --git a/vyper/venom/analysis/dup_requirements.py b/vyper/venom/analysis/dup_requirements.py deleted file mode 100644 index 7afb315035..0000000000 --- a/vyper/venom/analysis/dup_requirements.py +++ /dev/null @@ -1,15 +0,0 @@ -from vyper.utils import OrderedSet -from vyper.venom.analysis.analysis import IRAnalysis - - -class DupRequirementsAnalysis(IRAnalysis): - def analyze(self): - for bb in self.function.get_basic_blocks(): - last_liveness = bb.out_vars - for inst in reversed(bb.instructions): - inst.dup_requirements = OrderedSet() - ops = inst.get_input_variables() - for op in ops: - if op in last_liveness: - inst.dup_requirements.add(op) - last_liveness = inst.liveness diff --git a/vyper/venom/basicblock.py b/vyper/venom/basicblock.py index d6fb9560cd..1199579b3f 100644 --- a/vyper/venom/basicblock.py +++ b/vyper/venom/basicblock.py @@ -209,7 +209,6 @@ class IRInstruction: output: Optional[IROperand] # set of live variables at this instruction liveness: OrderedSet[IRVariable] - dup_requirements: OrderedSet[IRVariable] parent: "IRBasicBlock" fence_id: int annotation: Optional[str] @@ -228,7 +227,6 @@ def __init__( self.operands = list(operands) # in case we get an iterator self.output = output self.liveness = OrderedSet() - self.dup_requirements = OrderedSet() self.fence_id = -1 self.annotation = None self.ast_source = None diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 41a76319d7..390fab8e7c 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -12,7 +12,6 @@ ) from vyper.utils import MemoryPositions, OrderedSet from vyper.venom.analysis.analysis import IRAnalysesCache -from vyper.venom.analysis.dup_requirements import DupRequirementsAnalysis from vyper.venom.analysis.liveness import LivenessAnalysis from vyper.venom.basicblock import ( IRBasicBlock, @@ -153,7 +152,6 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: NormalizationPass(ac, fn).run_pass() self.liveness_analysis = ac.request_analysis(LivenessAnalysis) - ac.request_analysis(DupRequirementsAnalysis) assert fn.normalized, "Non-normalized CFG!" @@ -231,7 +229,12 @@ def _stack_reorder( return cost def _emit_input_operands( - self, assembly: list, inst: IRInstruction, ops: list[IROperand], stack: StackModel + self, + assembly: list, + inst: IRInstruction, + ops: list[IROperand], + stack: StackModel, + next_liveness: OrderedSet[IRVariable], ) -> None: # PRE: we already have all the items on the stack that have # been scheduled to be killed. now it's just a matter of emitting @@ -241,7 +244,7 @@ def _emit_input_operands( # it with something that is wanted if ops and stack.height > 0 and stack.peek(0) not in ops: for op in ops: - if isinstance(op, IRVariable) and op not in inst.dup_requirements: + if isinstance(op, IRVariable) and op not in next_liveness: self.swap_op(assembly, stack, op) break @@ -264,7 +267,7 @@ def _emit_input_operands( stack.push(op) continue - if op in inst.dup_requirements and op not in emitted_ops: + if op in next_liveness and op not in emitted_ops: self.dup_op(assembly, stack, op) if op in emitted_ops: @@ -288,7 +291,9 @@ def _generate_evm_for_basicblock_r( all_insts = sorted(basicblock.instructions, key=lambda x: x.opcode != "param") for i, inst in enumerate(all_insts): - next_liveness = all_insts[i + 1].liveness if i + 1 < len(all_insts) else OrderedSet() + next_liveness = ( + all_insts[i + 1].liveness if i + 1 < len(all_insts) else basicblock.out_vars + ) asm.extend(self._generate_evm_for_instruction(inst, stack, next_liveness)) @@ -327,10 +332,9 @@ def clean_stack_from_cfg_in( self.pop(asm, stack) def _generate_evm_for_instruction( - self, inst: IRInstruction, stack: StackModel, next_liveness: OrderedSet = None + self, inst: IRInstruction, stack: StackModel, next_liveness: OrderedSet ) -> list[str]: assembly: list[str | int] = [] - next_liveness = next_liveness or OrderedSet() opcode = inst.opcode # @@ -375,7 +379,7 @@ def _generate_evm_for_instruction( # example, for `%56 = %label1 %13 %label2 %14`, we will # find an instance of %13 *or* %14 in the stack and replace it with %56. to_be_replaced = stack.peek(depth) - if to_be_replaced in inst.dup_requirements: + if to_be_replaced in next_liveness: # %13/%14 is still live(!), so we make a copy of it self.dup(assembly, stack, depth) stack.poke(0, ret) @@ -390,7 +394,7 @@ def _generate_evm_for_instruction( return apply_line_numbers(inst, assembly) # Step 2: Emit instruction's input operands - self._emit_input_operands(assembly, inst, operands, stack) + self._emit_input_operands(assembly, inst, operands, stack, next_liveness) # Step 3: Reorder stack before join points if opcode == "jmp":