From 4c8bc4572c2a0c3656ff8783bb03bbd37b2fed57 Mon Sep 17 00:00:00 2001 From: plodeada Date: Tue, 6 May 2025 19:42:58 +0200 Subject: [PATCH 01/47] start of the analysis --- vyper/venom/analysis/__init__.py | 1 + vyper/venom/analysis/stack_order.py | 87 +++++++++++++++++++++++++++++ vyper/venom/passes/dft.py | 18 +++++- 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 vyper/venom/analysis/stack_order.py diff --git a/vyper/venom/analysis/__init__.py b/vyper/venom/analysis/__init__.py index 2b6b722d7b..b7ffcd84e4 100644 --- a/vyper/venom/analysis/__init__.py +++ b/vyper/venom/analysis/__init__.py @@ -8,3 +8,4 @@ from .mem_ssa import MemSSA from .reachable import ReachableAnalysis from .var_definition import VarDefinition +from .stack_order import StackOrder diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py new file mode 100644 index 0000000000..55d6e29325 --- /dev/null +++ b/vyper/venom/analysis/stack_order.py @@ -0,0 +1,87 @@ +from vyper.venom.analysis.analysis import IRAnalysis +from vyper.venom.basicblock import IRBasicBlock, IROperand, IRVariable +from vyper.utils import OrderedSet, CompilerPanic +from vyper.venom.analysis import LivenessAnalysis + + +def swap(stack: list[IROperand], position: int): + if position == 0: + return + top = len(stack) - 1 + stack[top], stack[position] = stack[position], stack[top] + +def position(stack: list[IROperand], op: IROperand): + top = len(stack) - 1 + for i, item in enumerate(reversed(stack)): + pos = top - i + if item == op: + return pos + + raise CompilerPanic("item not in the stack") + +def ops_order(stack: list[IROperand], needed: list[IROperand], next_liveness: OrderedSet[IRVariable]) -> tuple[list[IROperand], list[IROperand]]: + added: list[IROperand] = [] + # we will have to dup these either way + for op in needed: + if op in next_liveness: + stack.append(op) + # pad stack with unknown + diff_count = len(needed) > len(stack) + if diff_count > 0: + stack = needed[0:diff_count] + stack + added = needed[0:diff_count] + + for op in needed: + if op not in stack: + stack.insert(0, op) + added.insert(0, op) + + for i, op in enumerate(needed): + pos = len(needed) - i - 1 + if op in next_liveness: + stack.append(op) + swap(stack, pos) + continue + if op == stack[pos]: + continue + current_pos = position(stack, op) + swap(stack, current_pos) + swap(stack, pos) + + + return stack, added + +class StackOrder(IRAnalysis): + bb_to_stack: dict[IRBasicBlock, list[IROperand]] + liveness: LivenessAnalysis + + def analyze(self): + self.bb_to_stack = dict() + self.liveness = self.analyses_cache.request_analysis(LivenessAnalysis) + for bb in self.function.get_basic_blocks(): + self.bb_to_stack[bb] = self._handle_bb(bb) + + def _handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: + stack: list[IROperand] = [] + needed: list[IROperand] = [] + + for i, inst in enumerate(bb.instructions): + print(inst) + print(stack) + ops = inst.operands + if inst.is_bb_terminator: + next_liveness = self.liveness.out_vars(inst.parent) + else: + next_inst = inst.parent.instructions[i + 1] + next_liveness = self.liveness.inst_to_liveness[next_inst] + stack, added = ops_order(stack, ops, next_liveness) + print(stack) + needed = added + needed + assert stack[-len(ops):] == ops, (stack, ops) + stack = stack[0:(-len(ops))] + if inst.output is not None: + stack.append(inst.output) + + return needed + + diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index 02570bb403..084ea18e1a 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -2,7 +2,7 @@ import vyper.venom.effects as effects from vyper.utils import OrderedSet -from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis +from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, StackOrder, CFGAnalysis from vyper.venom.basicblock import IRBasicBlock, IRInstruction from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass @@ -17,9 +17,15 @@ class DFTPass(IRPass): # "effect dependency analysis" eda: dict[IRInstruction, OrderedSet[IRInstruction]] + stack_order: StackOrder + cfg: CFGAnalysis + + def run_pass(self) -> None: self.data_offspring = {} self.visited_instructions: OrderedSet[IRInstruction] = OrderedSet() + self.stack_order = self.analyses_cache.request_analysis(StackOrder) + self.cfg = self.analyses_cache.request_analysis(CFGAnalysis) self.dfg = self.analyses_cache.force_analysis(DFGAnalysis) @@ -97,6 +103,16 @@ def _calculate_dependency_graphs(self, bb: IRBasicBlock) -> None: last_read_effects: dict[effects.Effects, IRInstruction] = {} for inst in non_phis: + if inst.opcode == "jmp": + next_bbs = self.cfg.cfg_out(bb) + assert len(next_bbs) == 1 + next_bb = next_bbs.first() + stack_vars = self.stack_order.bb_to_stack[next_bb] + for op in stack_vars: + dep = self.dfg.get_producing_instruction(op) + if dep is not None and dep.parent == bb: + self.dda[inst].add(dep) + continue for op in inst.operands: dep = self.dfg.get_producing_instruction(op) if dep is not None and dep.parent == bb: From 239fc650913c34ad498554a49fdd1246112c906f Mon Sep 17 00:00:00 2001 From: plodeada Date: Wed, 7 May 2025 09:22:04 +0200 Subject: [PATCH 02/47] only single use not rename --- vyper/venom/passes/dft.py | 6 +++--- vyper/venom/passes/single_use_expansion.py | 13 ++++++++++++- vyper/venom/venom_to_assembly.py | 2 +- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index 084ea18e1a..a4821aac88 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -17,14 +17,14 @@ class DFTPass(IRPass): # "effect dependency analysis" eda: dict[IRInstruction, OrderedSet[IRInstruction]] - stack_order: StackOrder + #stack_order: StackOrder cfg: CFGAnalysis def run_pass(self) -> None: self.data_offspring = {} self.visited_instructions: OrderedSet[IRInstruction] = OrderedSet() - self.stack_order = self.analyses_cache.request_analysis(StackOrder) + #self.stack_order = self.analyses_cache.request_analysis(StackOrder) self.cfg = self.analyses_cache.request_analysis(CFGAnalysis) self.dfg = self.analyses_cache.force_analysis(DFGAnalysis) @@ -103,7 +103,7 @@ def _calculate_dependency_graphs(self, bb: IRBasicBlock) -> None: last_read_effects: dict[effects.Effects, IRInstruction] = {} for inst in non_phis: - if inst.opcode == "jmp": + if False and inst.opcode == "jmp": next_bbs = self.cfg.cfg_out(bb) assert len(next_bbs) == 1 next_bb = next_bbs.first() diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 50dc16aea6..c81d2403d0 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -20,6 +20,7 @@ class SingleUseExpansion(IRPass): """ def run_pass(self): + self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) for bb in self.function.get_basic_blocks(): self._process_bb(bb) @@ -39,7 +40,17 @@ def _process_bb(self, bb): if inst.opcode == "log" and j == 0: continue - if isinstance(op, (IRVariable, IRLiteral)): + if isinstance(op, IRVariable): + uses = self.dfg.get_uses(op) + if len(uses) == 1 and len([x for x in inst.operands if x == op]) == 1: + continue + var = self.function.get_next_variable() + to_insert = IRInstruction("store", [op], var) + bb.insert_instruction(to_insert, index=i) + inst.operands[j] = var + i += 1 + + if isinstance(op, IRLiteral): var = self.function.get_next_variable() to_insert = IRInstruction("store", [op], var) bb.insert_instruction(to_insert, index=i) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 4c5a2bfcda..2a53dfb1a1 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -279,7 +279,7 @@ def _emit_input_operands( self.dup_op(assembly, stack, op) # guaranteed by store expansion - assert op not in seen, (op, seen) + assert op not in seen, (inst, op, seen) seen.add(op) def _prepare_stack_for_function(self, asm, fn: IRFunction, stack: StackModel): From c22817ec3feba18a6a5b93eae918c427dcdd2baf Mon Sep 17 00:00:00 2001 From: plodeada Date: Thu, 8 May 2025 13:04:41 +0200 Subject: [PATCH 03/47] start of the swap type check --- vyper/venom/analysis/stack_order.py | 31 +++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 55d6e29325..8bfc20f974 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -1,7 +1,8 @@ from vyper.venom.analysis.analysis import IRAnalysis -from vyper.venom.basicblock import IRBasicBlock, IROperand, IRVariable +from vyper.venom.basicblock import IRBasicBlock, IROperand, IRVariable, IRInstruction, IRLiteral from vyper.utils import OrderedSet, CompilerPanic from vyper.venom.analysis import LivenessAnalysis +from enum import Enum def swap(stack: list[IROperand], position: int): @@ -51,13 +52,22 @@ def ops_order(stack: list[IROperand], needed: list[IROperand], next_liveness: Or return stack, added +class StoreType(Enum): + PUSH = 1 + SWAP = 2 + DUP = 3 + class StackOrder(IRAnalysis): bb_to_stack: dict[IRBasicBlock, list[IROperand]] liveness: LivenessAnalysis + store_to_type: dict[IRInstruction, StoreType] def analyze(self): - self.bb_to_stack = dict() self.liveness = self.analyses_cache.request_analysis(LivenessAnalysis) + self.store_to_type = dict() + for bb in self.function.get_basic_blocks(): + self._handle_bb_store_types(bb) + self.bb_to_stack = dict() for bb in self.function.get_basic_blocks(): self.bb_to_stack[bb] = self._handle_bb(bb) @@ -84,4 +94,21 @@ def _handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: return needed + def _handle_bb_store_types(self, bb: IRBasicBlock): + for i, inst in enumerate(bb.instructions): + if inst.opcode != "store": + continue + op = inst.operands[0] + if isinstance(op, IRLiteral): + self.store_to_type[inst] = StoreType.PUSH + continue + + assert isinstance(op, IRVariable) + next_liveness = self.liveness.live_vars_at(bb.instructions[i + 1]) + if op in next_liveness: + self.store_to_type[inst] = StoreType.DUP + else: + self.store_to_type[inst] = StoreType.SWAP + + From d6e403f2ad528a8025dc0b37fe0a480a435453b3 Mon Sep 17 00:00:00 2001 From: plodeada Date: Mon, 12 May 2025 09:40:55 +0200 Subject: [PATCH 04/47] handling stores --- vyper/venom/analysis/stack_order.py | 88 +++++++++++++---------------- 1 file changed, 39 insertions(+), 49 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 8bfc20f974..5e9d6e055a 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -5,52 +5,21 @@ from enum import Enum -def swap(stack: list[IROperand], position: int): +def swap(stack: list[IROperand], position: int, output: IRVariable): if position == 0: return top = len(stack) - 1 stack[top], stack[position] = stack[position], stack[top] + stack[top] = output -def position(stack: list[IROperand], op: IROperand): +def position(stack: list[IROperand], op: IROperand) -> int | None: top = len(stack) - 1 for i, item in enumerate(reversed(stack)): pos = top - i if item == op: return pos - raise CompilerPanic("item not in the stack") - -def ops_order(stack: list[IROperand], needed: list[IROperand], next_liveness: OrderedSet[IRVariable]) -> tuple[list[IROperand], list[IROperand]]: - added: list[IROperand] = [] - # we will have to dup these either way - for op in needed: - if op in next_liveness: - stack.append(op) - # pad stack with unknown - diff_count = len(needed) > len(stack) - if diff_count > 0: - stack = needed[0:diff_count] + stack - added = needed[0:diff_count] - - for op in needed: - if op not in stack: - stack.insert(0, op) - added.insert(0, op) - - for i, op in enumerate(needed): - pos = len(needed) - i - 1 - if op in next_liveness: - stack.append(op) - swap(stack, pos) - continue - if op == stack[pos]: - continue - current_pos = position(stack, op) - swap(stack, current_pos) - swap(stack, pos) - - - return stack, added + return None class StoreType(Enum): PUSH = 1 @@ -76,24 +45,45 @@ def _handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: needed: list[IROperand] = [] for i, inst in enumerate(bb.instructions): - print(inst) - print(stack) - ops = inst.operands - if inst.is_bb_terminator: - next_liveness = self.liveness.out_vars(inst.parent) + if inst.opcode == "store": + self._handle_store(inst, stack, needed) else: - next_inst = inst.parent.instructions[i + 1] - next_liveness = self.liveness.inst_to_liveness[next_inst] - stack, added = ops_order(stack, ops, next_liveness) - print(stack) - needed = added + needed - assert stack[-len(ops):] == ops, (stack, ops) - stack = stack[0:(-len(ops))] - if inst.output is not None: - stack.append(inst.output) + ops = inst.operands + assert ops == stack[-len(ops)] + + stack = stack[0:-len(ops)] + + output = inst.output + if output is not None: + stack.append(output) return needed + def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: list[IROperand]): + assert inst.opcode == "store" + ops = inst.operands + assert len(ops) == 0 + op = ops[0] + + output = inst.output + assert output is not None + + store_type = self.store_to_type[inst] + if store_type == StoreType.PUSH: + stack.append(output) + elif store_type == StoreType.SWAP: + op_position = position(stack, op) + if op_position is None: + stack.insert(0, op) + needed.append(op) + op_position = len(stack) - 1 + swap(stack, op_position, output) + elif store_type == StoreType.DUP: + stack.append(output) + if op not in stack: + needed.append(op) + + def _handle_bb_store_types(self, bb: IRBasicBlock): for i, inst in enumerate(bb.instructions): if inst.opcode != "store": From 193ef3eb7772b387cceceb0f9a39035bd4dba644 Mon Sep 17 00:00:00 2001 From: Hodan Date: Thu, 15 May 2025 13:35:12 +0200 Subject: [PATCH 05/47] it does something --- vyper/venom/analysis/stack_order.py | 94 +++++++++++++++++++++++------ vyper/venom/passes/dft.py | 42 +++++++------ 2 files changed, 99 insertions(+), 37 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 5e9d6e055a..04edcdda48 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -1,57 +1,104 @@ from vyper.venom.analysis.analysis import IRAnalysis -from vyper.venom.basicblock import IRBasicBlock, IROperand, IRVariable, IRInstruction, IRLiteral +from vyper.venom.basicblock import IRBasicBlock, IROperand, IRVariable, IRInstruction, IRLiteral, IRLabel +from vyper.venom.function import IRFunction from vyper.utils import OrderedSet, CompilerPanic from vyper.venom.analysis import LivenessAnalysis +from vyper.venom.analysis.analysis import IRAnalysesCache from enum import Enum -def swap(stack: list[IROperand], position: int, output: IRVariable): - if position == 0: - return +def swap(stack: list[IROperand], position: int, output: IROperand | None = None): top = len(stack) - 1 + position = top - position + if position == top: + if output is not None: + stack[top] = output + return stack[top], stack[position] = stack[position], stack[top] - stack[top] = output + if output is not None: + stack[top] = output + +def top(stack: list[IROperand]) -> IROperand: + top_idx = len(stack) - 1 + return stack[top_idx] def position(stack: list[IROperand], op: IROperand) -> int | None: top = len(stack) - 1 - for i, item in enumerate(reversed(stack)): + for i, item in enumerate(stack): pos = top - i if item == op: return pos return None +def op_reorder(stack: list[IROperand], ops: list[IROperand]) -> list[IROperand]: + needed: list[IROperand] = [] + for op in reversed(ops): + if op not in stack: + needed.append(op) + stack.insert(0, op) + for i, op in reversed(list(enumerate(reversed(ops)))): + assert op in stack + op_position = position(stack, op) + assert op_position is not None, f"operand is not in stack {op}, {stack}" + + #assert isinstance(op, IRVariable), f"operand must be variable got {op}" + swap(stack, op_position) + swap(stack, i) + return needed + +def max_same_prefix(stack_a: list[IROperand], stack_b: list[IROperand]) -> list[IROperand]: + res = [] + for a, b in zip(reversed(stack_a), reversed(stack_b)): + if a != b: + break + res.append(a) + return res + class StoreType(Enum): PUSH = 1 SWAP = 2 DUP = 3 -class StackOrder(IRAnalysis): +# this wont be part of the analysis framework +class StackOrder: bb_to_stack: dict[IRBasicBlock, list[IROperand]] liveness: LivenessAnalysis store_to_type: dict[IRInstruction, StoreType] - def analyze(self): + def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): + self.analyses_cache = analyses_cache + self.function = function + self.store_to_type = dict() self.liveness = self.analyses_cache.request_analysis(LivenessAnalysis) + + def calculates_store_types(self): self.store_to_type = dict() for bb in self.function.get_basic_blocks(): self._handle_bb_store_types(bb) - self.bb_to_stack = dict() - for bb in self.function.get_basic_blocks(): - self.bb_to_stack[bb] = self._handle_bb(bb) - def _handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: + def handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: stack: list[IROperand] = [] needed: list[IROperand] = [] - for i, inst in enumerate(bb.instructions): + for inst in bb.instructions: if inst.opcode == "store": self._handle_store(inst, stack, needed) else: - ops = inst.operands - assert ops == stack[-len(ops)] + if inst.opcode == "phi": + ops = [op for _, op in inst.phi_operands] + elif inst.is_bb_terminator: + ops = [op for op in inst.operands if not isinstance(op, IRLabel)] + else: + ops = inst.operands + + inst_needed = op_reorder(stack, ops) + needed.extend(inst_needed) + if len(ops) > 0: + stack_top = list(stack[-len(ops):]) + assert ops == stack_top, f"the top of the stack is not correct, {ops}, {stack_top}" - stack = stack[0:-len(ops)] + stack = stack[0:-len(ops)] output = inst.output if output is not None: @@ -59,10 +106,21 @@ def _handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: return needed + def handle_bbs(self, bbs: list[IRBasicBlock]) -> list[IROperand]: + if len(bbs) == 0: + return [] + res = self.handle_bb(bbs[0]) + for bb in bbs[1:]: + tmp = self.handle_bb(bb) + res = max_same_prefix(res, tmp) + return list(reversed(res)) + + + def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: list[IROperand]): assert inst.opcode == "store" ops = inst.operands - assert len(ops) == 0 + assert len(ops) == 1 op = ops[0] output = inst.output @@ -76,7 +134,7 @@ def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: lis if op_position is None: stack.insert(0, op) needed.append(op) - op_position = len(stack) - 1 + op_position = 0 swap(stack, op_position, output) elif store_type == StoreType.DUP: stack.append(output) diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index a4821aac88..1b250e4d1d 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -3,7 +3,7 @@ import vyper.venom.effects as effects from vyper.utils import OrderedSet from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, StackOrder, CFGAnalysis -from vyper.venom.basicblock import IRBasicBlock, IRInstruction +from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IROperand from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass @@ -17,25 +17,31 @@ class DFTPass(IRPass): # "effect dependency analysis" eda: dict[IRInstruction, OrderedSet[IRInstruction]] - #stack_order: StackOrder + stack_order: StackOrder cfg: CFGAnalysis def run_pass(self) -> None: self.data_offspring = {} self.visited_instructions: OrderedSet[IRInstruction] = OrderedSet() - #self.stack_order = self.analyses_cache.request_analysis(StackOrder) + self.stack_order = StackOrder(self.analyses_cache, self.function) self.cfg = self.analyses_cache.request_analysis(CFGAnalysis) self.dfg = self.analyses_cache.force_analysis(DFGAnalysis) - for bb in self.function.get_basic_blocks(): - self._process_basic_block(bb) + self.stack_order.calculates_store_types() + + for bb in self.cfg.dfs_post_walk: + stack_order = self.stack_order.handle_bbs(list(self.cfg.cfg_out(bb))) + self._process_basic_block(bb, stack_order) + + #for bb in self.function.get_basic_blocks(): + #self._process_basic_block(bb) self.analyses_cache.invalidate_analysis(LivenessAnalysis) - def _process_basic_block(self, bb: IRBasicBlock) -> None: - self._calculate_dependency_graphs(bb) + def _process_basic_block(self, bb: IRBasicBlock, stack_order: list[IROperand]) -> None: + self._calculate_dependency_graphs(bb, stack_order) self.instructions = list(bb.pseudo_instructions) non_phi_instructions = list(bb.non_phi_instructions) @@ -53,12 +59,11 @@ def _process_basic_block(self, bb: IRBasicBlock) -> None: self.visited_instructions = OrderedSet() for inst in entry_instructions_list: - self._process_instruction_r(self.instructions, inst) - + self._process_instruction_r(self.instructions, inst, stack_order) bb.instructions = self.instructions assert bb.is_terminated, f"Basic block should be terminated {bb}" - def _process_instruction_r(self, instructions: list[IRInstruction], inst: IRInstruction): + def _process_instruction_r(self, instructions: list[IRInstruction], inst: IRInstruction, stack_order: list[IROperand]): if inst in self.visited_instructions: return self.visited_instructions.add(inst) @@ -74,7 +79,10 @@ def cost(x: IRInstruction) -> int | float: else: assert x in self.dda[inst] # sanity check assert x.output is not None # help mypy - ret = inst.operands.index(x.output) + if inst.opcode == "jmp": + ret = stack_order.index(x.output) + else: + ret = inst.operands.index(x.output) return ret # heuristic: sort by size of child dependency graph @@ -85,11 +93,11 @@ def cost(x: IRInstruction) -> int | float: inst.flip() for dep_inst in children: - self._process_instruction_r(instructions, dep_inst) + self._process_instruction_r(instructions, dep_inst, stack_order) instructions.append(inst) - def _calculate_dependency_graphs(self, bb: IRBasicBlock) -> None: + def _calculate_dependency_graphs(self, bb: IRBasicBlock, out_stack: list[IROperand]) -> None: # ida: instruction dependency analysis self.dda = defaultdict(OrderedSet) self.eda = defaultdict(OrderedSet) @@ -103,12 +111,8 @@ def _calculate_dependency_graphs(self, bb: IRBasicBlock) -> None: last_read_effects: dict[effects.Effects, IRInstruction] = {} for inst in non_phis: - if False and inst.opcode == "jmp": - next_bbs = self.cfg.cfg_out(bb) - assert len(next_bbs) == 1 - next_bb = next_bbs.first() - stack_vars = self.stack_order.bb_to_stack[next_bb] - for op in stack_vars: + if inst.opcode == "jmp": + for op in out_stack: dep = self.dfg.get_producing_instruction(op) if dep is not None and dep.parent == bb: self.dda[inst].add(dep) From 1230890e09376f9d198d810f350321a952d8d940 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 16 May 2025 13:39:39 +0200 Subject: [PATCH 06/47] more terminators --- vyper/venom/analysis/stack_order.py | 8 +++++--- vyper/venom/passes/dft.py | 20 +++++++++++++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 04edcdda48..68d2ab140a 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -45,7 +45,7 @@ def op_reorder(stack: list[IROperand], ops: list[IROperand]) -> list[IROperand]: #assert isinstance(op, IRVariable), f"operand must be variable got {op}" swap(stack, op_position) swap(stack, i) - return needed + return list(reversed(needed)) def max_same_prefix(stack_a: list[IROperand], stack_b: list[IROperand]) -> list[IROperand]: res = [] @@ -53,7 +53,8 @@ def max_same_prefix(stack_a: list[IROperand], stack_b: list[IROperand]) -> list[ if a != b: break res.append(a) - return res + #print(res) + return list(reversed(res)) class StoreType(Enum): PUSH = 1 @@ -134,7 +135,8 @@ def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: lis if op_position is None: stack.insert(0, op) needed.append(op) - op_position = 0 + op_position = position(stack, op) + assert op_position is not None swap(stack, op_position, output) elif store_type == StoreType.DUP: stack.append(output) diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index 1b250e4d1d..5311019ffb 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -6,6 +6,7 @@ from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IROperand from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass +from collections import deque class DFTPass(IRPass): @@ -31,9 +32,22 @@ def run_pass(self) -> None: self.stack_order.calculates_store_types() - for bb in self.cfg.dfs_post_walk: + worklist = deque(self.cfg.dfs_post_walk) + last_stack_orders: dict[IRBasicBlock, list] = dict() + + while len(worklist) > 0: + bb = worklist.popleft() stack_order = self.stack_order.handle_bbs(list(self.cfg.cfg_out(bb))) + if bb in last_stack_orders and stack_order == last_stack_orders[bb]: + continue + last_stack_orders[bb] = stack_order self._process_basic_block(bb, stack_order) + for inbb in self.cfg.cfg_in(bb): + worklist.append(inbb) + + #for bb in self.cfg.dfs_post_walk: + #stack_order = self.stack_order.handle_bbs(list(self.cfg.cfg_out(bb))) + #self._process_basic_block(bb, stack_order) #for bb in self.function.get_basic_blocks(): #self._process_basic_block(bb) @@ -79,7 +93,7 @@ def cost(x: IRInstruction) -> int | float: else: assert x in self.dda[inst] # sanity check assert x.output is not None # help mypy - if inst.opcode == "jmp": + if inst.is_bb_terminator: ret = stack_order.index(x.output) else: ret = inst.operands.index(x.output) @@ -111,7 +125,7 @@ def _calculate_dependency_graphs(self, bb: IRBasicBlock, out_stack: list[IROpera last_read_effects: dict[effects.Effects, IRInstruction] = {} for inst in non_phis: - if inst.opcode == "jmp": + if inst.is_bb_terminator: for op in out_stack: dep = self.dfg.get_producing_instruction(op) if dep is not None and dep.parent == bb: From 2231fbe7518de4c8733e07d5a442fff2f844030d Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 19 May 2025 09:41:45 +0200 Subject: [PATCH 07/47] fix for label test and lint --- vyper/venom/analysis/__init__.py | 2 +- vyper/venom/analysis/stack_order.py | 52 +++++++++++++++++------------ vyper/venom/passes/dft.py | 20 +++++------ 3 files changed, 41 insertions(+), 33 deletions(-) diff --git a/vyper/venom/analysis/__init__.py b/vyper/venom/analysis/__init__.py index b7ffcd84e4..978fbdeb76 100644 --- a/vyper/venom/analysis/__init__.py +++ b/vyper/venom/analysis/__init__.py @@ -7,5 +7,5 @@ from .mem_alias import MemoryAliasAnalysis from .mem_ssa import MemSSA from .reachable import ReachableAnalysis -from .var_definition import VarDefinition from .stack_order import StackOrder +from .var_definition import VarDefinition diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 68d2ab140a..4a29fea075 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -1,10 +1,16 @@ -from vyper.venom.analysis.analysis import IRAnalysis -from vyper.venom.basicblock import IRBasicBlock, IROperand, IRVariable, IRInstruction, IRLiteral, IRLabel -from vyper.venom.function import IRFunction -from vyper.utils import OrderedSet, CompilerPanic +from enum import Enum + from vyper.venom.analysis import LivenessAnalysis from vyper.venom.analysis.analysis import IRAnalysesCache -from enum import Enum +from vyper.venom.basicblock import ( + IRBasicBlock, + IRInstruction, + IRLabel, + IRLiteral, + IROperand, + IRVariable, +) +from vyper.venom.function import IRFunction def swap(stack: list[IROperand], position: int, output: IROperand | None = None): @@ -18,10 +24,12 @@ def swap(stack: list[IROperand], position: int, output: IROperand | None = None) if output is not None: stack[top] = output + def top(stack: list[IROperand]) -> IROperand: top_idx = len(stack) - 1 return stack[top_idx] + def position(stack: list[IROperand], op: IROperand) -> int | None: top = len(stack) - 1 for i, item in enumerate(stack): @@ -31,6 +39,7 @@ def position(stack: list[IROperand], op: IROperand) -> int | None: return None + def op_reorder(stack: list[IROperand], ops: list[IROperand]) -> list[IROperand]: needed: list[IROperand] = [] for op in reversed(ops): @@ -41,26 +50,29 @@ def op_reorder(stack: list[IROperand], ops: list[IROperand]) -> list[IROperand]: assert op in stack op_position = position(stack, op) assert op_position is not None, f"operand is not in stack {op}, {stack}" - - #assert isinstance(op, IRVariable), f"operand must be variable got {op}" + + # assert isinstance(op, IRVariable), f"operand must be variable got {op}" swap(stack, op_position) swap(stack, i) return list(reversed(needed)) + def max_same_prefix(stack_a: list[IROperand], stack_b: list[IROperand]) -> list[IROperand]: res = [] for a, b in zip(reversed(stack_a), reversed(stack_b)): if a != b: break res.append(a) - #print(res) + # print(res) return list(reversed(res)) + class StoreType(Enum): PUSH = 1 SWAP = 2 DUP = 3 + # this wont be part of the analysis framework class StackOrder: bb_to_stack: dict[IRBasicBlock, list[IROperand]] @@ -78,7 +90,7 @@ def calculates_store_types(self): for bb in self.function.get_basic_blocks(): self._handle_bb_store_types(bb) - def handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: + def handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: stack: list[IROperand] = [] needed: list[IROperand] = [] @@ -94,12 +106,14 @@ def handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: ops = inst.operands inst_needed = op_reorder(stack, ops) - needed.extend(inst_needed) + needed.extend(inst_needed) if len(ops) > 0: - stack_top = list(stack[-len(ops):]) - assert ops == stack_top, f"the top of the stack is not correct, {ops}, {stack_top}" + stack_top = list(stack[-len(ops) :]) + assert ( + ops == stack_top + ), f"the top of the stack is not correct, {ops}, {stack_top}" - stack = stack[0:-len(ops)] + stack = stack[0 : -len(ops)] output = inst.output if output is not None: @@ -116,8 +130,6 @@ def handle_bbs(self, bbs: list[IRBasicBlock]) -> list[IROperand]: res = max_same_prefix(res, tmp) return list(reversed(res)) - - def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: list[IROperand]): assert inst.opcode == "store" ops = inst.operands @@ -126,7 +138,7 @@ def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: lis output = inst.output assert output is not None - + store_type = self.store_to_type[inst] if store_type == StoreType.PUSH: stack.append(output) @@ -143,13 +155,12 @@ def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: lis if op not in stack: needed.append(op) - def _handle_bb_store_types(self, bb: IRBasicBlock): for i, inst in enumerate(bb.instructions): if inst.opcode != "store": continue - op = inst.operands[0] - if isinstance(op, IRLiteral): + op = inst.operands[0] + if isinstance(op, (IRLiteral, IRLabel)): self.store_to_type[inst] = StoreType.PUSH continue @@ -159,6 +170,3 @@ def _handle_bb_store_types(self, bb: IRBasicBlock): self.store_to_type[inst] = StoreType.DUP else: self.store_to_type[inst] = StoreType.SWAP - - - diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index 5311019ffb..afe0d50e53 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -1,12 +1,11 @@ -from collections import defaultdict +from collections import defaultdict, deque import vyper.venom.effects as effects from vyper.utils import OrderedSet -from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, StackOrder, CFGAnalysis +from vyper.venom.analysis import CFGAnalysis, DFGAnalysis, LivenessAnalysis, StackOrder from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IROperand from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass -from collections import deque class DFTPass(IRPass): @@ -21,7 +20,6 @@ class DFTPass(IRPass): stack_order: StackOrder cfg: CFGAnalysis - def run_pass(self) -> None: self.data_offspring = {} self.visited_instructions: OrderedSet[IRInstruction] = OrderedSet() @@ -45,12 +43,12 @@ def run_pass(self) -> None: for inbb in self.cfg.cfg_in(bb): worklist.append(inbb) - #for bb in self.cfg.dfs_post_walk: - #stack_order = self.stack_order.handle_bbs(list(self.cfg.cfg_out(bb))) - #self._process_basic_block(bb, stack_order) + # for bb in self.cfg.dfs_post_walk: + # stack_order = self.stack_order.handle_bbs(list(self.cfg.cfg_out(bb))) + # self._process_basic_block(bb, stack_order) - #for bb in self.function.get_basic_blocks(): - #self._process_basic_block(bb) + # for bb in self.function.get_basic_blocks(): + # self._process_basic_block(bb) self.analyses_cache.invalidate_analysis(LivenessAnalysis) @@ -77,7 +75,9 @@ def _process_basic_block(self, bb: IRBasicBlock, stack_order: list[IROperand]) - bb.instructions = self.instructions assert bb.is_terminated, f"Basic block should be terminated {bb}" - def _process_instruction_r(self, instructions: list[IRInstruction], inst: IRInstruction, stack_order: list[IROperand]): + def _process_instruction_r( + self, instructions: list[IRInstruction], inst: IRInstruction, stack_order: list[IROperand] + ): if inst in self.visited_instructions: return self.visited_instructions.add(inst) From 899067711e5c283d327618dbc74edbd968e7b2c3 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 19 May 2025 10:43:27 +0200 Subject: [PATCH 08/47] start of the test, correct terminators with ops and small refactors --- tests/unit/compiler/venom/test_stack_order.py | 60 +++++++++++++++++++ vyper/venom/analysis/stack_order.py | 10 ++-- vyper/venom/passes/dft.py | 13 ++-- 3 files changed, 73 insertions(+), 10 deletions(-) create mode 100644 tests/unit/compiler/venom/test_stack_order.py diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py new file mode 100644 index 0000000000..9ebdfce9a5 --- /dev/null +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -0,0 +1,60 @@ +from tests.venom_utils import PrePostChecker, parse_from_basic_block +from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.passes import AssignElimination, DFTPass, SimplifyCFGPass, SingleUseExpansion +from vyper.venom.venom_to_assembly import VenomCompiler + +# assing elim is there to have easier check +_check_pre_post = PrePostChecker([SingleUseExpansion, DFTPass, AssignElimination]) + + +def test_stack_order_basic(): + pre = """ + main: + %1 = mload 1 + %2 = mload 2 + jmp @next + next: + %3 = add 1, %1 + %4 = add 1, %2 + return %4, %3 + """ + + post = """ + main: + %2 = mload 2 + %1 = mload 1 + jmp @next + next: + %3 = add 1, %1 + %4 = add 1, %2 + return %4, %3 + """ + + _check_pre_post(pre, post) + + ctx = parse_from_basic_block(post) + for fn in ctx.get_functions(): + ac = IRAnalysesCache(fn) + SingleUseExpansion(ac, fn).run_pass() + SimplifyCFGPass(ac, fn).run_pass() + + print(ctx) + + asm = VenomCompiler([ctx]).generate_evm() + print(asm) + assert asm == [ + "PUSH1", + 2, + "MLOAD", + "PUSH1", + 1, + "MLOAD", + "PUSH1", + 1, + "ADD", + "SWAP1", # swap out the result of the first add (only necessary swap) + "PUSH1", + 1, + "ADD", + "RETURN", + ] diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 4a29fea075..9c7d607eee 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -54,17 +54,17 @@ def op_reorder(stack: list[IROperand], ops: list[IROperand]) -> list[IROperand]: # assert isinstance(op, IRVariable), f"operand must be variable got {op}" swap(stack, op_position) swap(stack, i) - return list(reversed(needed)) + return needed def max_same_prefix(stack_a: list[IROperand], stack_b: list[IROperand]) -> list[IROperand]: res = [] - for a, b in zip(reversed(stack_a), reversed(stack_b)): + for a, b in zip(stack_a, stack_b): if a != b: break res.append(a) # print(res) - return list(reversed(res)) + return res class StoreType(Enum): @@ -119,7 +119,7 @@ def handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: if output is not None: stack.append(output) - return needed + return list(reversed(needed)) def handle_bbs(self, bbs: list[IRBasicBlock]) -> list[IROperand]: if len(bbs) == 0: @@ -128,7 +128,7 @@ def handle_bbs(self, bbs: list[IRBasicBlock]) -> list[IROperand]: for bb in bbs[1:]: tmp = self.handle_bb(bb) res = max_same_prefix(res, tmp) - return list(reversed(res)) + return res def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: list[IROperand]): assert inst.opcode == "store" diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index afe0d50e53..2381b9e3dc 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -94,7 +94,10 @@ def cost(x: IRInstruction) -> int | float: assert x in self.dda[inst] # sanity check assert x.output is not None # help mypy if inst.is_bb_terminator: - ret = stack_order.index(x.output) + if x.output in inst.operands: + ret = inst.operands.index(x.output) + else: + ret = stack_order.index(x.output) else: ret = inst.operands.index(x.output) return ret @@ -125,16 +128,16 @@ def _calculate_dependency_graphs(self, bb: IRBasicBlock, out_stack: list[IROpera last_read_effects: dict[effects.Effects, IRInstruction] = {} for inst in non_phis: + for op in inst.operands: + dep = self.dfg.get_producing_instruction(op) + if dep is not None and dep.parent == bb: + self.dda[inst].add(dep) if inst.is_bb_terminator: for op in out_stack: dep = self.dfg.get_producing_instruction(op) if dep is not None and dep.parent == bb: self.dda[inst].add(dep) continue - for op in inst.operands: - dep = self.dfg.get_producing_instruction(op) - if dep is not None and dep.parent == bb: - self.dda[inst].add(dep) write_effects = inst.get_write_effects() read_effects = inst.get_read_effects() From d88b4a47b15bb91295cc428eb063e9c3a0761932 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 19 May 2025 11:06:54 +0200 Subject: [PATCH 09/47] reverse test to basic --- tests/unit/compiler/venom/test_stack_order.py | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index 9ebdfce9a5..48b6075cf3 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -58,3 +58,56 @@ def test_stack_order_basic(): "ADD", "RETURN", ] + + +def test_stack_order_basic2(): + pre = """ + main: + %1 = mload 1 + %2 = mload 2 + jmp @next + next: + %3 = add 1, %1 + %4 = add 1, %2 + return %3, %4 + """ + + post = """ + main: + %1 = mload 1 + %2 = mload 2 + jmp @next + next: + %4 = add 1, %2 + %3 = add 1, %1 + return %3, %4 + """ + + _check_pre_post(pre, post) + + ctx = parse_from_basic_block(post) + for fn in ctx.get_functions(): + ac = IRAnalysesCache(fn) + SingleUseExpansion(ac, fn).run_pass() + SimplifyCFGPass(ac, fn).run_pass() + + print(ctx) + + asm = VenomCompiler([ctx]).generate_evm() + print(asm) + assert asm == [ + "PUSH1", + 1, + "MLOAD", + "PUSH1", + 2, + "MLOAD", + "PUSH1", + 1, + "ADD", + "SWAP1", # swap out the result of the first add (only necessary swap) + "PUSH1", + 1, + "ADD", + "RETURN", + ] From 9f53ae6c29072485b632d6ca823f44504b9fc0eb Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 19 May 2025 12:40:08 +0200 Subject: [PATCH 10/47] small cleanups --- vyper/venom/analysis/analysis.py | 2 +- vyper/venom/passes/dft.py | 17 ++++------------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/vyper/venom/analysis/analysis.py b/vyper/venom/analysis/analysis.py index 7ee6499720..cfcf188427 100644 --- a/vyper/venom/analysis/analysis.py +++ b/vyper/venom/analysis/analysis.py @@ -75,7 +75,7 @@ def invalidate_analysis(self, analysis_cls: Type[IRAnalysis]): if analysis is not None: analysis.invalidate() - def force_analysis(self, analysis_cls: Type[IRAnalysis], *args, **kwargs): + def force_analysis(self, analysis_cls: Type[T], *args, **kwargs) -> T: """ Force a specific analysis to be run on the IR even if it has already been run, and is cached. diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index 2381b9e3dc..df3d221206 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -43,13 +43,6 @@ def run_pass(self) -> None: for inbb in self.cfg.cfg_in(bb): worklist.append(inbb) - # for bb in self.cfg.dfs_post_walk: - # stack_order = self.stack_order.handle_bbs(list(self.cfg.cfg_out(bb))) - # self._process_basic_block(bb, stack_order) - - # for bb in self.function.get_basic_blocks(): - # self._process_basic_block(bb) - self.analyses_cache.invalidate_analysis(LivenessAnalysis) def _process_basic_block(self, bb: IRBasicBlock, stack_order: list[IROperand]) -> None: @@ -93,13 +86,11 @@ def cost(x: IRInstruction) -> int | float: else: assert x in self.dda[inst] # sanity check assert x.output is not None # help mypy - if inst.is_bb_terminator: - if x.output in inst.operands: - ret = inst.operands.index(x.output) - else: - ret = stack_order.index(x.output) - else: + if x.output in inst.operands: ret = inst.operands.index(x.output) + else: + assert inst.is_bb_terminator + ret = stack_order.index(x.output) return ret # heuristic: sort by size of child dependency graph From df28b35b4fdcb9c7ce2367f7bea863a1b8a8bff7 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 19 May 2025 14:28:43 +0200 Subject: [PATCH 11/47] removed stray commented code --- vyper/venom/analysis/stack_order.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 9c7d607eee..f3a763a8c4 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -51,7 +51,6 @@ def op_reorder(stack: list[IROperand], ops: list[IROperand]) -> list[IROperand]: op_position = position(stack, op) assert op_position is not None, f"operand is not in stack {op}, {stack}" - # assert isinstance(op, IRVariable), f"operand must be variable got {op}" swap(stack, op_position) swap(stack, i) return needed @@ -63,7 +62,6 @@ def max_same_prefix(stack_a: list[IROperand], stack_b: list[IROperand]) -> list[ if a != b: break res.append(a) - # print(res) return res From 988debbd8c1e7eb9a48ab7399c6661ad79e65e99 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 19 May 2025 15:19:30 +0200 Subject: [PATCH 12/47] more correct behaviour for jnz --- tests/unit/compiler/venom/test_stack_order.py | 38 +++++++++++++++++++ vyper/venom/passes/dft.py | 13 +++---- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index 48b6075cf3..93ae23932a 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -111,3 +111,41 @@ def test_stack_order_basic2(): "ADD", "RETURN", ] + + +def test_stack_reorder_split(): + pre = """ + main: + %1 = mload 1 + %2 = mload 2 + %3 = add 1, %2 + %cond = mload 3 + jnz %cond, @then, @else + then: + %4a = add 1, %1 + %5a = add 1, %3 + sink %5a, %4a + else: + %4b = add 1, %1 + %5b = add 1, %3 + sink %5b, %4b + """ + + post = """ + main: + %2 = mload 2 + %3 = add 1, %2 + %1 = mload 1 + %cond = mload 3 + jnz %cond, @then, @else + then: + %4a = add 1, %1 + %5a = add 1, %3 + sink %5a, %4a + else: + %4b = add 1, %1 + %5b = add 1, %3 + sink %5b, %4b + """ + + _check_pre_post(pre, post) diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index df3d221206..e4476c569b 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -81,13 +81,13 @@ def _process_instruction_r( children = list(self.dda[inst] | self.eda[inst]) def cost(x: IRInstruction) -> int | float: - if x in self.eda[inst] or inst.flippable: + if (x not in self.dda[inst] and x in self.eda[inst]) or inst.flippable: ret = -1 * int(len(self.data_offspring[x]) > 0) else: assert x in self.dda[inst] # sanity check assert x.output is not None # help mypy if x.output in inst.operands: - ret = inst.operands.index(x.output) + ret = inst.operands.index(x.output) + len(stack_order) else: assert inst.is_bb_terminator ret = stack_order.index(x.output) @@ -119,16 +119,15 @@ def _calculate_dependency_graphs(self, bb: IRBasicBlock, out_stack: list[IROpera last_read_effects: dict[effects.Effects, IRInstruction] = {} for inst in non_phis: - for op in inst.operands: - dep = self.dfg.get_producing_instruction(op) - if dep is not None and dep.parent == bb: - self.dda[inst].add(dep) if inst.is_bb_terminator: for op in out_stack: dep = self.dfg.get_producing_instruction(op) if dep is not None and dep.parent == bb: self.dda[inst].add(dep) - continue + for op in inst.operands: + dep = self.dfg.get_producing_instruction(op) + if dep is not None and dep.parent == bb: + self.dda[inst].add(dep) write_effects = inst.get_write_effects() read_effects = inst.get_read_effects() From 588c91f3067cd22cd865befef4349370479567a3 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 19 May 2025 16:38:50 +0200 Subject: [PATCH 13/47] propagate order in analysis --- vyper/venom/analysis/stack_order.py | 30 +++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index f3a763a8c4..4cf3abb7a0 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -1,6 +1,6 @@ from enum import Enum -from vyper.venom.analysis import LivenessAnalysis +from vyper.venom.analysis import LivenessAnalysis, CFGAnalysis from vyper.venom.analysis.analysis import IRAnalysesCache from vyper.venom.basicblock import ( IRBasicBlock, @@ -11,6 +11,7 @@ IRVariable, ) from vyper.venom.function import IRFunction +from typing import Iterator def swap(stack: list[IROperand], position: int, output: IROperand | None = None): @@ -81,6 +82,8 @@ def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): self.analyses_cache = analyses_cache self.function = function self.store_to_type = dict() + self.bb_to_stack = dict() + self.cfg = self.analyses_cache.request_analysis(CFGAnalysis) self.liveness = self.analyses_cache.request_analysis(LivenessAnalysis) def calculates_store_types(self): @@ -100,6 +103,12 @@ def handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: ops = [op for _, op in inst.phi_operands] elif inst.is_bb_terminator: ops = [op for op in inst.operands if not isinstance(op, IRLabel)] + out_bbs = self.cfg.cfg_out(bb) + orders = [self.bb_to_stack.get(out_bb, []) for out_bb in out_bbs] + tmp = self.merge(orders) + for op in reversed(tmp): + if op not in ops: + ops.append(op) else: ops = inst.operands @@ -117,17 +126,22 @@ def handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: if output is not None: stack.append(output) - return list(reversed(needed)) + res = list(reversed(needed)) + self.bb_to_stack[bb] = res + return res - def handle_bbs(self, bbs: list[IRBasicBlock]) -> list[IROperand]: - if len(bbs) == 0: + def merge(self, orders: list[list[IROperand]]) -> list[IROperand]: + if len(orders) == 0: return [] - res = self.handle_bb(bbs[0]) - for bb in bbs[1:]: - tmp = self.handle_bb(bb) - res = max_same_prefix(res, tmp) + res: list[IROperand] = orders[0].copy() + for order in orders: + res = max_same_prefix(res, order) return res + def handle_bbs(self, bbs: list[IRBasicBlock]) -> list[IROperand]: + bb_orders = [self.handle_bb(bb) for bb in bbs] + return self.merge(bb_orders) + def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: list[IROperand]): assert inst.opcode == "store" ops = inst.operands From 803f376a61ea2109fc79d0ec5082aa7eeb5054ae Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 19 May 2025 19:24:29 +0200 Subject: [PATCH 14/47] test for join and lint --- tests/unit/compiler/venom/test_stack_order.py | 40 +++++++++++++++++++ vyper/venom/analysis/stack_order.py | 3 +- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index 93ae23932a..f6a2ab547d 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -149,3 +149,43 @@ def test_stack_reorder_split(): """ _check_pre_post(pre, post) + + +def test_stack_order_join(): + pre = """ + main: + %cond = param + %1 = mload 1 + %2 = mload 2 + jnz %cond, @then, @else + then: + %3a = mload 3 + mstore 1000, %3a + jmp @join + else: + %3b = mload 3 + mstore 2000, %3b + jmp @join + join: + sink %1 + """ + + post = """ + main: + %cond = param + %2 = mload 2 + %1 = mload 1 + jnz %cond, @then, @else + then: + %3a = mload 3 + mstore 1000, %3a + jmp @join + else: + %3b = mload 3 + mstore 2000, %3b + jmp @join + join: + sink %1 + """ + + _check_pre_post(pre, post) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 4cf3abb7a0..82842a4169 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -1,6 +1,6 @@ from enum import Enum -from vyper.venom.analysis import LivenessAnalysis, CFGAnalysis +from vyper.venom.analysis import CFGAnalysis, LivenessAnalysis from vyper.venom.analysis.analysis import IRAnalysesCache from vyper.venom.basicblock import ( IRBasicBlock, @@ -11,7 +11,6 @@ IRVariable, ) from vyper.venom.function import IRFunction -from typing import Iterator def swap(stack: list[IROperand], position: int, output: IROperand | None = None): From f8e04ca461803d2d59534c2513e7620c6db0bc41 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 20 May 2025 09:27:37 +0200 Subject: [PATCH 15/47] test rename --- tests/unit/compiler/venom/test_stack_order.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index f6a2ab547d..030aeb6f15 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -113,7 +113,7 @@ def test_stack_order_basic2(): ] -def test_stack_reorder_split(): +def test_stack_order_split(): pre = """ main: %1 = mload 1 From 3f36ddb8d3de7a1385e7a7a926940df44d5113cc Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 20 May 2025 10:41:45 +0200 Subject: [PATCH 16/47] merge test --- tests/unit/compiler/venom/test_stack_order.py | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index 030aeb6f15..1994c0ddc5 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -150,6 +150,43 @@ def test_stack_order_split(): _check_pre_post(pre, post) +def test_stack_order_split2(): + pre = """ + main: + %1 = mload 1 + %2 = mload 2 + %3 = add 1, %2 + %cond = mload 3 + jnz %cond, @then, @else + then: + %4a = add 1, %1 + %5a = add 1, %2 + sink %5a, %4a + else: + %4b = add 1, %1 + %5b = add 1, %3 + sink %5b, %4b + """ + + post = """ + main: + %2 = mload 2 + %3 = add 1, %2 + %1 = mload 1 + %cond = mload 3 + jnz %cond, @then, @else + then: + %4a = add 1, %1 + %5a = add 1, %2 + sink %5a, %4a + else: + %4b = add 1, %1 + %5b = add 1, %3 + sink %5b, %4b + """ + + _check_pre_post(pre, post) + def test_stack_order_join(): pre = """ From 68ecefd013b320a67d7ec972c1e69e6d1a621aec Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 20 May 2025 10:41:59 +0200 Subject: [PATCH 17/47] renames and correct order for both analysis and dft --- vyper/venom/analysis/stack_order.py | 22 +++++++++++++--------- vyper/venom/passes/dft.py | 2 +- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 82842a4169..0c281ac54e 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -53,7 +53,7 @@ def op_reorder(stack: list[IROperand], ops: list[IROperand]) -> list[IROperand]: swap(stack, op_position) swap(stack, i) - return needed + return list(reversed(needed)) def max_same_prefix(stack_a: list[IROperand], stack_b: list[IROperand]) -> list[IROperand]: @@ -90,7 +90,7 @@ def calculates_store_types(self): for bb in self.function.get_basic_blocks(): self._handle_bb_store_types(bb) - def handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: + def _handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: stack: list[IROperand] = [] needed: list[IROperand] = [] @@ -104,8 +104,8 @@ def handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: ops = [op for op in inst.operands if not isinstance(op, IRLabel)] out_bbs = self.cfg.cfg_out(bb) orders = [self.bb_to_stack.get(out_bb, []) for out_bb in out_bbs] - tmp = self.merge(orders) - for op in reversed(tmp): + tmp = self._merge(orders) + for op in tmp: if op not in ops: ops.append(op) else: @@ -125,11 +125,11 @@ def handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: if output is not None: stack.append(output) - res = list(reversed(needed)) + res = needed #list(reversed(needed)) self.bb_to_stack[bb] = res return res - def merge(self, orders: list[list[IROperand]]) -> list[IROperand]: + def _merge(self, orders: list[list[IROperand]]) -> list[IROperand]: if len(orders) == 0: return [] res: list[IROperand] = orders[0].copy() @@ -137,9 +137,13 @@ def merge(self, orders: list[list[IROperand]]) -> list[IROperand]: res = max_same_prefix(res, order) return res - def handle_bbs(self, bbs: list[IRBasicBlock]) -> list[IROperand]: - bb_orders = [self.handle_bb(bb) for bb in bbs] - return self.merge(bb_orders) + def get_prefered_stack(self, succesors: list[IRBasicBlock]) -> list[IROperand]: + bb_orders = [self._handle_bb(bb) for bb in succesors] + + # reverse so it it is in the same order + # as the operands for the easier handle + # in dft pass + return list(reversed(self._merge(bb_orders))) def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: list[IROperand]): assert inst.opcode == "store" diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index e4476c569b..53ab766b42 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -35,7 +35,7 @@ def run_pass(self) -> None: while len(worklist) > 0: bb = worklist.popleft() - stack_order = self.stack_order.handle_bbs(list(self.cfg.cfg_out(bb))) + stack_order = self.stack_order.get_prefered_stack(list(self.cfg.cfg_out(bb))) if bb in last_stack_orders and stack_order == last_stack_orders[bb]: continue last_stack_orders[bb] = stack_order From 0e80de2d412ff68d2fe3d500c54ab1bfb197afe8 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 20 May 2025 10:44:12 +0200 Subject: [PATCH 18/47] lint --- tests/unit/compiler/venom/test_stack_order.py | 1 + vyper/venom/analysis/stack_order.py | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index 1994c0ddc5..1a4fba63e6 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -150,6 +150,7 @@ def test_stack_order_split(): _check_pre_post(pre, post) + def test_stack_order_split2(): pre = """ main: diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 0c281ac54e..8f52b29268 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -125,9 +125,8 @@ def _handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: if output is not None: stack.append(output) - res = needed #list(reversed(needed)) - self.bb_to_stack[bb] = res - return res + self.bb_to_stack[bb] = needed + return needed def _merge(self, orders: list[list[IROperand]]) -> list[IROperand]: if len(orders) == 0: From 8926ab7253de3ecd70e503791da52fb629762357 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 20 May 2025 10:53:33 +0200 Subject: [PATCH 19/47] comments, small cleanup and lint --- vyper/venom/analysis/stack_order.py | 30 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 8f52b29268..d69b9f855b 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -13,7 +13,7 @@ from vyper.venom.function import IRFunction -def swap(stack: list[IROperand], position: int, output: IROperand | None = None): +def _swap(stack: list[IROperand], position: int, output: IROperand | None = None): top = len(stack) - 1 position = top - position if position == top: @@ -25,12 +25,7 @@ def swap(stack: list[IROperand], position: int, output: IROperand | None = None) stack[top] = output -def top(stack: list[IROperand]) -> IROperand: - top_idx = len(stack) - 1 - return stack[top_idx] - - -def position(stack: list[IROperand], op: IROperand) -> int | None: +def _position(stack: list[IROperand], op: IROperand) -> int | None: top = len(stack) - 1 for i, item in enumerate(stack): pos = top - i @@ -40,7 +35,7 @@ def position(stack: list[IROperand], op: IROperand) -> int | None: return None -def op_reorder(stack: list[IROperand], ops: list[IROperand]) -> list[IROperand]: +def _op_reorder(stack: list[IROperand], ops: list[IROperand]) -> list[IROperand]: needed: list[IROperand] = [] for op in reversed(ops): if op not in stack: @@ -48,11 +43,11 @@ def op_reorder(stack: list[IROperand], ops: list[IROperand]) -> list[IROperand]: stack.insert(0, op) for i, op in reversed(list(enumerate(reversed(ops)))): assert op in stack - op_position = position(stack, op) + op_position = _position(stack, op) assert op_position is not None, f"operand is not in stack {op}, {stack}" - swap(stack, op_position) - swap(stack, i) + _swap(stack, op_position) + _swap(stack, i) return list(reversed(needed)) @@ -111,7 +106,7 @@ def _handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: else: ops = inst.operands - inst_needed = op_reorder(stack, ops) + inst_needed = _op_reorder(stack, ops) needed.extend(inst_needed) if len(ops) > 0: stack_top = list(stack[-len(ops) :]) @@ -128,6 +123,8 @@ def _handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: self.bb_to_stack[bb] = needed return needed + # compute max prefix for all the orders of the + # succesor basicblocks def _merge(self, orders: list[list[IROperand]]) -> list[IROperand]: if len(orders) == 0: return [] @@ -141,7 +138,7 @@ def get_prefered_stack(self, succesors: list[IRBasicBlock]) -> list[IROperand]: # reverse so it it is in the same order # as the operands for the easier handle - # in dft pass + # in dft pass (same logic as normal inst) return list(reversed(self._merge(bb_orders))) def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: list[IROperand]): @@ -157,18 +154,19 @@ def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: lis if store_type == StoreType.PUSH: stack.append(output) elif store_type == StoreType.SWAP: - op_position = position(stack, op) + op_position = _position(stack, op) if op_position is None: stack.insert(0, op) needed.append(op) - op_position = position(stack, op) + op_position = _position(stack, op) assert op_position is not None - swap(stack, op_position, output) + _swap(stack, op_position, output) elif store_type == StoreType.DUP: stack.append(output) if op not in stack: needed.append(op) + # compute what will store do in bytecode def _handle_bb_store_types(self, bb: IRBasicBlock): for i, inst in enumerate(bb.instructions): if inst.opcode != "store": From c2de1b62442b310bf503973ce71530a1e47d3f01 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 20 May 2025 11:20:49 +0200 Subject: [PATCH 20/47] rename --- vyper/venom/analysis/stack_order.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index d69b9f855b..d1fbfacddb 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -51,7 +51,7 @@ def _op_reorder(stack: list[IROperand], ops: list[IROperand]) -> list[IROperand] return list(reversed(needed)) -def max_same_prefix(stack_a: list[IROperand], stack_b: list[IROperand]) -> list[IROperand]: +def _max_same_prefix(stack_a: list[IROperand], stack_b: list[IROperand]) -> list[IROperand]: res = [] for a, b in zip(stack_a, stack_b): if a != b: @@ -130,7 +130,7 @@ def _merge(self, orders: list[list[IROperand]]) -> list[IROperand]: return [] res: list[IROperand] = orders[0].copy() for order in orders: - res = max_same_prefix(res, order) + res = _max_same_prefix(res, order) return res def get_prefered_stack(self, succesors: list[IRBasicBlock]) -> list[IROperand]: From c336f8977ff238406161844c629ef76f586aea60 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 20 May 2025 11:36:41 +0200 Subject: [PATCH 21/47] non mergable test --- tests/unit/compiler/venom/test_stack_order.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index 1a4fba63e6..cc256f031a 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -6,6 +6,8 @@ # assing elim is there to have easier check _check_pre_post = PrePostChecker([SingleUseExpansion, DFTPass, AssignElimination]) +def _check_no_change(pre): + _check_pre_post(pre, pre, hevm=False) def test_stack_order_basic(): pre = """ @@ -227,3 +229,23 @@ def test_stack_order_join(): """ _check_pre_post(pre, post) + + +def test_stack_order_join_unmergable_stacks(): + pre = """ + main: + %cond = param + %1 = mload 1 + %2 = mload 2 + jnz %cond, @then, @else + then: + mstore 1000, %1 + jmp @join + else: + mstore 2000, %2 + jmp @join + join: + sink %1 + """ + + _check_no_change(pre) From 1b180bb83604981dd73b8e3f4fe7bfbeff7a4e5e Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 20 May 2025 13:36:07 +0200 Subject: [PATCH 22/47] more phi test and lint --- tests/unit/compiler/venom/test_stack_order.py | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index cc256f031a..dd79c88882 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -6,9 +6,11 @@ # assing elim is there to have easier check _check_pre_post = PrePostChecker([SingleUseExpansion, DFTPass, AssignElimination]) + def _check_no_change(pre): _check_pre_post(pre, pre, hevm=False) + def test_stack_order_basic(): pre = """ main: @@ -249,3 +251,91 @@ def test_stack_order_join_unmergable_stacks(): """ _check_no_change(pre) + + +def test_stack_order_phi(): + pre = """ + main: + %par = param + jnz %par, @then, @else + then: + %1a = add 1, %par + %2a = mload 10 + mstore 1000, %2a + jmp @join + else: + %1b = add 2, %par + %2b = mload 10 + mstore 1000, %2b + jmp @join + join: + %1 = phi @then, %1a, @else, %1b + %res = add 1, %1 ; properly use the value + sink %res + """ + + post = """ + main: + %par = param + jnz %par, @then, @else + then: + %2a = mload 10 + mstore 1000, %2a + %1a = add 1, %par + jmp @join + else: + %2b = mload 10 + mstore 1000, %2b + %1b = add 2, %par + jmp @join + join: + %1 = phi @then, %1a, @else, %1b + %res = add 1, %1 ; properly use the value + sink %res + """ + + _check_pre_post(pre, post) + + +def test_stack_order_more_phi(): + pre = """ + main: + %par = param + jnz %par, @then, @else + then: + %1a = add 1, %par + %2a = add 2, %par + jmp @join + else: + %1b = add 3, %par + %2b = add 4, %par + jmp @join + join: + %2 = phi @then, %2a, @else, %2b + %1 = phi @then, %1a, @else, %1b + %res1 = add 1, %1 ; properly use the value + %res2 = add 1, %2 ; properly use the value + sink %res1, %res2 + """ + + post = """ + main: + %par = param + jnz %par, @then, @else + then: + %2a = add 2, %par + %1a = add 1, %par + jmp @join + else: + %2b = add 4, %par + %1b = add 3, %par + jmp @join + join: + %2 = phi @then, %2a, @else, %2b + %1 = phi @then, %1a, @else, %1b + %res1 = add 1, %1 ; properly use the value + %res2 = add 1, %2 ; properly use the value + sink %res2, %res1 + """ + + _check_pre_post(pre, post) From dcbc2c3017e76a6c78849d877c2fbee4a7bfea04 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 20 May 2025 16:40:42 +0200 Subject: [PATCH 23/47] more phi test fix and xfail mark --- tests/unit/compiler/venom/test_stack_order.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index dd79c88882..cb24ed034e 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -1,3 +1,5 @@ +import pytest + from tests.venom_utils import PrePostChecker, parse_from_basic_block from vyper.venom.analysis.analysis import IRAnalysesCache from vyper.venom.passes import AssignElimination, DFTPass, SimplifyCFGPass, SingleUseExpansion @@ -297,6 +299,8 @@ def test_stack_order_phi(): _check_pre_post(pre, post) +# TODO: fix this xfail before merge +@pytest.mark.xfail def test_stack_order_more_phi(): pre = """ main: @@ -315,7 +319,7 @@ def test_stack_order_more_phi(): %1 = phi @then, %1a, @else, %1b %res1 = add 1, %1 ; properly use the value %res2 = add 1, %2 ; properly use the value - sink %res1, %res2 + sink %res2, %res1 """ post = """ From a4a8d5910a946ffc4c4814c703212a0e2fea095d Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 26 May 2025 13:25:50 +0200 Subject: [PATCH 24/47] fix phis in inliner --- vyper/venom/passes/function_inliner.py | 28 ++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/vyper/venom/passes/function_inliner.py b/vyper/venom/passes/function_inliner.py index 1c33e81d27..cfc27c1027 100644 --- a/vyper/venom/passes/function_inliner.py +++ b/vyper/venom/passes/function_inliner.py @@ -90,8 +90,9 @@ def _inline_function(self, func: IRFunction, call_sites: List[IRInstruction]) -> """ for call_site in call_sites: FloatAllocas(self.analyses_caches[func], func).run_pass() - self._inline_call_site(func, call_site) + return_bb = self._inline_call_site(func, call_site) fn = call_site.parent.parent + self._fix_phi(fn, call_site.parent, return_bb) self.analyses_caches[fn].invalidate_analysis(DFGAnalysis) self.analyses_caches[fn].invalidate_analysis(CFGAnalysis) @@ -144,7 +145,7 @@ def _inline_function(self, func: IRFunction, call_sites: List[IRInstruction]) -> # demote to alloca so that mem2var will work inst.opcode = "alloca" - def _inline_call_site(self, func: IRFunction, call_site: IRInstruction) -> None: + def _inline_call_site(self, func: IRFunction, call_site: IRInstruction) -> IRBasicBlock: """ Inline function into call site. """ @@ -206,6 +207,29 @@ def _inline_call_site(self, func: IRFunction, call_site: IRInstruction) -> None: call_site_bb.instructions = call_site_bb.instructions[:call_idx] call_site_bb.append_instruction("jmp", func_copy.entry.label) + return call_site_return + + def _fix_phi(self, fn: IRFunction, orig: IRBasicBlock, new: IRBasicBlock) -> None: + orig_label = orig.label + new_label = new.label + + next_bbs = [] + last_inst = new.instructions[-1] + if last_inst.opcode == "jmp": + next_bbs = [fn.get_basic_block(last_inst.operands[0].name)] + if last_inst.opcode == "jnz": + next_bbs = [ + fn.get_basic_block(last_inst.operands[1].name), + fn.get_basic_block(last_inst.operands[2].name), + ] + + for bb in next_bbs: + for inst in bb.instructions: + if inst.opcode != "phi": + continue + for i in range(len(inst.operands)): + if inst.operands[i].name == orig_label.name: + inst.operands[i] = new_label def _build_call_walk(self, function: IRFunction) -> OrderedSet[IRFunction]: """ From c7c479e25b6766e7ed2f863fb6320290d677ad4a Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 30 May 2025 10:06:06 +0200 Subject: [PATCH 25/47] trying to create better phi handle --- tests/unit/compiler/venom/test_stack_order.py | 2 +- vyper/venom/analysis/stack_order.py | 245 ++++++++++++------ vyper/venom/passes/dft.py | 2 +- vyper/venom/venom_to_assembly.py | 27 +- 4 files changed, 197 insertions(+), 79 deletions(-) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index cb24ed034e..99201731b8 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -300,7 +300,7 @@ def test_stack_order_phi(): # TODO: fix this xfail before merge -@pytest.mark.xfail +#@pytest.mark.xfail def test_stack_order_more_phi(): pre = """ main: diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index d1fbfacddb..7202afdac6 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -11,44 +11,9 @@ IRVariable, ) from vyper.venom.function import IRFunction +from collections import deque, defaultdict -def _swap(stack: list[IROperand], position: int, output: IROperand | None = None): - top = len(stack) - 1 - position = top - position - if position == top: - if output is not None: - stack[top] = output - return - stack[top], stack[position] = stack[position], stack[top] - if output is not None: - stack[top] = output - - -def _position(stack: list[IROperand], op: IROperand) -> int | None: - top = len(stack) - 1 - for i, item in enumerate(stack): - pos = top - i - if item == op: - return pos - - return None - - -def _op_reorder(stack: list[IROperand], ops: list[IROperand]) -> list[IROperand]: - needed: list[IROperand] = [] - for op in reversed(ops): - if op not in stack: - needed.append(op) - stack.insert(0, op) - for i, op in reversed(list(enumerate(reversed(ops)))): - assert op in stack - op_position = _position(stack, op) - assert op_position is not None, f"operand is not in stack {op}, {stack}" - - _swap(stack, op_position) - _swap(stack, i) - return list(reversed(needed)) def _max_same_prefix(stack_a: list[IROperand], stack_b: list[IROperand]) -> list[IROperand]: @@ -65,9 +30,58 @@ class StoreType(Enum): SWAP = 2 DUP = 3 +class Stack: + data: list[IROperand] + + def __init__(self): + self.data = [] + + def __repr__(self) -> str: + return repr(self.data) + + def _swap(self, position: int, output: IROperand | None = None): + top = len(self.data) - 1 + position = top - position + if position == top: + if output is not None: + self.data[top] = output + return + self.data[top], self.data[position] = self.data[position], self.data[top] + if output is not None: + self.data[top] = output + + + def _position(self, op: IROperand) -> int | None: + top = len(self.data) - 1 + for i, item in enumerate(self.data): + pos = top - i + if item == op: + return pos + + return None + + + def _op_reorder(self, ops: list[IROperand]) -> list[IROperand]: + needed: list[IROperand] = [] + for op in reversed(ops): + if op not in self.data: + if op not in needed: + needed.append(op) + self.data.insert(0, op) + for i, op in reversed(list(enumerate(reversed(ops)))): + assert op in self.data + op_position = self._position(op) + assert op_position is not None, f"operand is not in stack {op}, {self.data}" + + self._swap(op_position) + self._swap(i) + return list(reversed(needed)) + + # this wont be part of the analysis framework class StackOrder: + from_to_stack: dict[tuple[IRBasicBlock, IRBasicBlock], list[IROperand]] bb_to_stack: dict[IRBasicBlock, list[IROperand]] liveness: LivenessAnalysis store_to_type: dict[IRInstruction, StoreType] @@ -77,6 +91,7 @@ def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): self.function = function self.store_to_type = dict() self.bb_to_stack = dict() + self.from_to_stack = dict() self.cfg = self.analyses_cache.request_analysis(CFGAnalysis) self.liveness = self.analyses_cache.request_analysis(LivenessAnalysis) @@ -85,40 +100,68 @@ def calculates_store_types(self): for bb in self.function.get_basic_blocks(): self._handle_bb_store_types(bb) + def calculate_all_orders(self): + self.calculates_store_types() + worklist = deque(self.cfg.dfs_post_walk) + last_stack_orders: dict[IRBasicBlock, list] = dict() + + while len(worklist) > 0: + bb = worklist.popleft() + stack_order = self.get_prefered_stack(bb, list(self.cfg.cfg_out(bb))) + if bb in last_stack_orders and stack_order == last_stack_orders[bb]: + continue + last_stack_orders[bb] = stack_order + for inbb in self.cfg.cfg_in(bb): + worklist.append(inbb) + + def _handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: - stack: list[IROperand] = [] + stack: Stack = Stack() needed: list[IROperand] = [] + phis: set[IRVariable] = set() + # from bb we need this var and it is rewriten to that var + phi_renames: dict[IRLabel, list[tuple[IRVariable, IRVariable, IRInstruction]]] = defaultdict(list) + phi_positions: dict[IRVariable, int] = dict() for inst in bb.instructions: if inst.opcode == "store": - self._handle_store(inst, stack, needed) + inst_needed = self._handle_store(inst, stack) + elif inst.opcode == "phi": + inst_needed = self._handle_phi(inst, phis, phi_renames) else: - if inst.opcode == "phi": - ops = [op for _, op in inst.phi_operands] - elif inst.is_bb_terminator: - ops = [op for op in inst.operands if not isinstance(op, IRLabel)] - out_bbs = self.cfg.cfg_out(bb) - orders = [self.bb_to_stack.get(out_bb, []) for out_bb in out_bbs] - tmp = self._merge(orders) - for op in tmp: - if op not in ops: - ops.append(op) - else: - ops = inst.operands - - inst_needed = _op_reorder(stack, ops) - needed.extend(inst_needed) - if len(ops) > 0: - stack_top = list(stack[-len(ops) :]) - assert ( - ops == stack_top - ), f"the top of the stack is not correct, {ops}, {stack_top}" - - stack = stack[0 : -len(ops)] - - output = inst.output - if output is not None: - stack.append(output) + inst_needed = self._handle_other_inst(inst, stack) + + for op in inst_needed: + if op not in needed: + if op in phis: + assert op not in phi_positions + assert isinstance(op, IRVariable) + phi_positions[op] = len(needed) + needed.append(op) + + + for pred in self.cfg.cfg_in(bb): + assert isinstance(pred, IRBasicBlock) # help mypy + transition = needed.copy() + removed = 0 + added = [] + for var, phi_var, inst in phi_renames[pred.label]: + assert var in self.liveness.out_vars(pred), f"{var} not life at {pred}, {inst}, {bb}, {self.liveness.out_vars(pred)}" + if phi_var not in phi_positions: + added.append(var) + continue + #assert phi_var in phi_positions, f"phi var not in phi positions {phi_var}, {phi_positions}, {bb}" + pos = phi_positions[phi_var] - removed + if var in transition: + del transition[pos] + removed += 1 + continue + + assert transition[pos] == phi_var + transition[pos] = var + transition = added + transition + self.from_to_stack[(pred, bb)] = transition + self.bb_to_stack[bb] = needed return needed @@ -133,15 +176,18 @@ def _merge(self, orders: list[list[IROperand]]) -> list[IROperand]: res = _max_same_prefix(res, order) return res - def get_prefered_stack(self, succesors: list[IRBasicBlock]) -> list[IROperand]: - bb_orders = [self._handle_bb(bb) for bb in succesors] + def get_prefered_stack(self, origin: IRBasicBlock, succesors: list[IRBasicBlock]) -> list[IROperand]: + for bb in succesors: + self._handle_bb(bb) + bb_orders = [self.from_to_stack[(origin, bb)] for bb in succesors] # reverse so it it is in the same order # as the operands for the easier handle # in dft pass (same logic as normal inst) return list(reversed(self._merge(bb_orders))) - def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: list[IROperand]): + def _handle_store(self, inst: IRInstruction, stack: Stack) -> list[IROperand]: + needed = [] assert inst.opcode == "store" ops = inst.operands assert len(ops) == 1 @@ -152,20 +198,71 @@ def _handle_store(self, inst: IRInstruction, stack: list[IROperand], needed: lis store_type = self.store_to_type[inst] if store_type == StoreType.PUSH: - stack.append(output) + stack.data.append(output) elif store_type == StoreType.SWAP: - op_position = _position(stack, op) + op_position = stack._position(op) if op_position is None: - stack.insert(0, op) + stack.data.insert(0, op) + #assert op not in needed, f"op {op} already in needed {needed} ({inst})" needed.append(op) - op_position = _position(stack, op) + op_position = stack._position(op) assert op_position is not None - _swap(stack, op_position, output) + stack._swap(op_position, output) elif store_type == StoreType.DUP: - stack.append(output) - if op not in stack: + stack.data.append(output) + if op not in stack.data: needed.append(op) + return needed + + def _handle_other_inst(self, inst: IRInstruction, stack: Stack) -> list[IROperand]: + assert inst.opcode not in ("store", "phi") + bb = inst.parent + if inst.is_bb_terminator: + ops = [op for op in inst.operands if not isinstance(op, IRLabel)] + out_bbs = self.cfg.cfg_out(bb) + orders = [self.from_to_stack.get((bb, out_bb), []) for out_bb in out_bbs] + tmp = self._merge(orders) + for op in tmp: + if op not in ops: + ops.append(op) + elif inst.opcode == "log": + # first opcode in log is magic + ops = inst.operands[1:] + elif inst.opcode == "offset": + # offset ops are magic + ops = [] + else: + ops = inst.operands + + ops = [op for op in ops if not isinstance(op, IRLabel)] + assert not any(isinstance(op, IRLiteral) for op in ops), inst + inst_needed = stack._op_reorder(ops) + + if len(ops) > 0: + stack_top = list(stack.data[-len(ops) :]) + assert ( + ops == stack_top + ), f"the top of the stack is not correct, {ops}, {stack_top}" + + stack.data = stack.data[0 : -len(ops)] + + if inst.output is not None: + stack.data.append(inst.output) + + return inst_needed + + def _handle_phi(self, inst: IRInstruction, phis: set[IRVariable], phi_renames) -> list[IROperand]: + assert inst.opcode == "phi" + assert inst.output is not None + for label, op in inst.phi_operands: + assert isinstance(op, IRVariable) + # phi renames op -> inst.output + phi_renames[label].append((op, inst.output, inst)) + #stack.append(inst.output) + phis.add(inst.output) + return [] + # compute what will store do in bytecode def _handle_bb_store_types(self, bb: IRBasicBlock): for i, inst in enumerate(bb.instructions): diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index 53ab766b42..b21d801b81 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -35,7 +35,7 @@ def run_pass(self) -> None: while len(worklist) > 0: bb = worklist.popleft() - stack_order = self.stack_order.get_prefered_stack(list(self.cfg.cfg_out(bb))) + stack_order = self.stack_order.get_prefered_stack(bb, list(self.cfg.cfg_out(bb))) if bb in last_stack_orders and stack_order == last_stack_orders[bb]: continue last_stack_orders[bb] = stack_order diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 2a53dfb1a1..5497b439f8 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -24,6 +24,7 @@ from vyper.venom.context import IRContext, IRFunction from vyper.venom.passes import NormalizationPass from vyper.venom.stack_model import StackModel +from vyper.venom.analysis import StackOrder DEBUG_SHOW_COST = False if DEBUG_SHOW_COST: @@ -161,6 +162,8 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: self.liveness = ac.request_analysis(LivenessAnalysis) self.dfg = ac.request_analysis(DFGAnalysis) self.cfg = ac.request_analysis(CFGAnalysis) + self.stack_order = StackOrder(ac, fn) + self.stack_order.calculate_all_orders() assert self.cfg.is_normalized(), "Non-normalized CFG!" @@ -214,7 +217,7 @@ def _stack_reorder( if len(stack_ops) == 0: return 0 - assert len(stack_ops) == len(set(stack_ops)) # precondition + assert len(stack_ops) == len(set(stack_ops)), f"duplicated stack {stack_ops}" # precondition cost = 0 for i, op in enumerate(stack_ops): @@ -470,10 +473,28 @@ def _generate_evm_for_instruction( # guaranteed by cfg normalization+simplification assert len(self.cfg.cfg_in(next_bb)) > 1 - target_stack = self.liveness.input_vars_from(inst.parent, next_bb) + liv = self.liveness.input_vars_from(inst.parent, next_bb) + target_stack = list(reversed(self.stack_order.from_to_stack[(inst.parent, next_bb)])) # NOTE: in general the stack can contain multiple copies of # the same variable, however, before a jump that is not possible - self._stack_reorder(assembly, stack, list(target_stack)) + import sys + #print(target_stack, list(liv + tmp = False + if target_stack != list(liv): + print(inst.parent, target_stack, list(liv), file=sys.stderr) + tmp = True + #for op in reversed(liv): + #if op not in target_stack: + #target_stack.insert(0, op) + + target_stack = list(self.liveness.input_vars_from(inst.parent, next_bb)) + + + if tmp: + print(stack, file=sys.stderr) + self._stack_reorder(assembly, stack, target_stack) + if tmp: + print(stack, assembly, file=sys.stderr) if inst.is_commutative: cost_no_swap = self._stack_reorder([], stack, operands, dry_run=True) From b126cea9a9eb25fe864c1cfab28a052caf5e8bb1 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 11 Jul 2025 14:14:00 +0200 Subject: [PATCH 26/47] quick rewrite try --- vyper/venom/analysis/stack_order.py | 151 +++++++++++++++++++++ vyper/venom/passes/dft.py | 36 ++++- vyper/venom/passes/single_use_expansion.py | 8 +- vyper/venom/venom_to_assembly.py | 9 +- 4 files changed, 189 insertions(+), 15 deletions(-) create mode 100644 vyper/venom/analysis/stack_order.py diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py new file mode 100644 index 0000000000..c7bcfdaeb2 --- /dev/null +++ b/vyper/venom/analysis/stack_order.py @@ -0,0 +1,151 @@ +from vyper.venom.basicblock import IROperand, IRBasicBlock, IRInstruction, IRVariable +from vyper.venom.function import IRFunction +from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.analysis import LivenessAnalysis, CFGAnalysis + +# needed [top, ... , bottom] +type Needed = list[IROperand] + +type Stack = list[IROperand] + +def _swap(stack: Stack, op: IROperand): + top = len(stack) - 1 + index = None + for i, item in reversed(list(enumerate(stack))): + if item == op: + index = i + + assert index is not None + + stack[index], stack[top] = stack[top], stack[index] + + +def _swap_to(stack: Stack, depth: int): + top = len(stack) - 1 + index = top - depth + + stack[index], stack[top] = stack[top], stack[index] + + +def _max_same_prefix(stack_a: Needed, stack_b: Needed): + res: list[IROperand] = [] + for a, b in zip(stack_a, stack_b): + if a != b: + break + res.append(a) + return res + +class StackOrderAnalysis: + function: IRFunction + liveness: LivenessAnalysis + cfg: CFGAnalysis + _from_to: dict[tuple[IRBasicBlock, IRBasicBlock], Needed] + + + def __init__(self, function: IRFunction, ac: IRAnalysesCache): + self._from_to = dict() + self.ac = ac + self.liveness = ac.request_analysis(LivenessAnalysis) + self.cfg = ac.request_analysis(CFGAnalysis) + + + def analyze_bb(self, bb: IRBasicBlock) -> Needed: + self.needed = [] + self.stack = [] + + for inst in bb.instructions: + if inst.opcode == "assign": + self._handle_assign(inst) + if inst.opcode == "phi": + self._handle_inst(inst) + elif inst.is_bb_terminator: + self._handle_terminator(inst) + else: + self._handle_inst(inst) + + for pred in self.cfg.cfg_in(bb): + self._from_to[(pred, bb)] = self.needed + + return self.needed + + def get_stack(self, bb: IRBasicBlock) -> Needed: + succs = self.cfg.cfg_out(bb) + orders = [self.from_to(bb, succ) for succ in succs] + return self._merge(orders) + + def from_to(self, origin: IRBasicBlock, successor: IRBasicBlock) -> Needed: + target = self._from_to.get((origin, successor), []).copy() + + for var in self.liveness.input_vars_from(origin, successor): + if var not in target: + target.append(var) + + return target + + def _handle_assign(self, inst: IRInstruction): + assert inst.opcode == "assign" + + self.liveness.live_vars_at(inst) + index = inst.parent.instructions.index(inst) + next_inst = inst.parent.instructions[index + 1] + next_live = self.liveness.live_vars_at(next_inst) + + src = inst.operands[0] + + if not isinstance(src, IRVariable): + self.stack.append(src) + elif src in next_live: + self.stack.append(src) + if src in self.stack: + self._add_needed(src) + else: + if src not in self.stack: + self.stack.append(src) + self._add_needed(src) + else: + _swap(self.stack, src) + + def _add_needed(self, op: IROperand): + if op not in self.needed: + self.needed.append(op) + + def _reorder(self, target_stack: Stack): + count = len(target_stack) + + for index, op in enumerate(target_stack): + depth = count - index - 1 + _swap(self.stack, op) + _swap_to(self.stack, depth) + self._add_needed(op) + + if len(target_stack) != 0: + assert target_stack == self.stack[-len(target_stack):], (target_stack, self.stack) + + + + def _handle_inst(self, inst: IRInstruction): + ops = inst.operands + for op in ops: + if isinstance(op, IRVariable): + self._add_needed(op) + if op not in self.stack: + self.stack.append(op) + self._reorder(ops) + + def _merge(self, orders: list[Needed]) -> Needed: + if len(orders) == 0: + return [] + res = orders[0] + for order in orders: + res = _max_same_prefix(res, order) + return res + + def _handle_terminator(self, inst: IRInstruction): + bb = inst.parent + orders = [self.from_to(bb, succ) for succ in self.cfg.cfg_out(bb)] + ops = (op for op in inst.operands if isinstance(op, IRVariable)) + for op in ops: + self._add_needed(op) + for op in self._merge(orders): + self._add_needed(op) + diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index fcb41b767e..d11e223c6f 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -1,9 +1,10 @@ -from collections import defaultdict +from collections import defaultdict, deque import vyper.venom.effects as effects from vyper.utils import OrderedSet -from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis -from vyper.venom.basicblock import IRBasicBlock, IRInstruction +from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, CFGAnalysis +from vyper.venom.analysis.stack_order import StackOrderAnalysis +from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IROperand from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass @@ -22,10 +23,23 @@ def run_pass(self) -> None: self.visited_instructions: OrderedSet[IRInstruction] = OrderedSet() self.dfg = self.analyses_cache.force_analysis(DFGAnalysis) + self.cfg = self.analyses_cache.request_analysis(CFGAnalysis) + self.stack_order = StackOrderAnalysis(self.function, self.analyses_cache) - for bb in self.function.get_basic_blocks(): + worklist = deque(self.cfg.dfs_post_walk) + + last_order: dict[IRBasicBlock, list[IROperand]] = dict() + + while len(worklist) > 0: + bb = worklist.popleft() + self.stack_order.analyze_bb(bb) + order = self.stack_order.get_stack(bb) + if bb in last_order and last_order[bb] == order: + break + self.order = order self._process_basic_block(bb) + self.analyses_cache.invalidate_analysis(LivenessAnalysis) def _process_basic_block(self, bb: IRBasicBlock) -> None: @@ -63,12 +77,17 @@ def _process_instruction_r(self, instructions: list[IRInstruction], inst: IRInst children = list(self.dda[inst] | self.eda[inst]) def cost(x: IRInstruction) -> int | float: - if x in self.eda[inst] or inst.flippable: + if (x not in self.dda[inst] and x in self.eda[inst]) or inst.flippable: ret = -1 * int(len(self.data_offspring[x]) > 0) + elif x.output in inst.operands: + assert x in self.dda[inst] # sanity check + assert x.output is not None # help mypy + ret = inst.operands.index(x.output) + len(self.order) else: assert x in self.dda[inst] # sanity check + assert x.output in self.order assert x.output is not None # help mypy - ret = inst.operands.index(x.output) + ret = self.order.index(x.output) return ret # heuristic: sort by size of child dependency graph @@ -97,6 +116,11 @@ def _calculate_dependency_graphs(self, bb: IRBasicBlock) -> None: all_read_effects: dict[effects.Effects, list[IRInstruction]] = defaultdict(list) for inst in non_phis: + if inst.is_bb_terminator: + for op in self.order: + dep = self.dfg.get_producing_instruction(op) + if dep is not None and dep.parent == bb: + self.dda[inst].add(dep) for op in inst.operands: dep = self.dfg.get_producing_instruction(op) if dep is not None and dep.parent == bb: diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index e50deed079..95910baf32 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -21,6 +21,7 @@ class SingleUseExpansion(IRPass): def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) + self.liveness = self.analyses_cache.request_analysis(LivenessAnalysis) for bb in self.function.get_basic_blocks(): self._process_bb(bb) @@ -35,7 +36,9 @@ def _process_bb(self, bb): i += 1 continue - for j, op in enumerate(inst.operands): + ops = inst.operands.copy() + + for j, op in enumerate(ops): # first operand to log is magic if inst.opcode == "log" and j == 0: continue @@ -54,7 +57,8 @@ def _process_bb(self, bb): var = self.function.get_next_variable() to_insert = IRInstruction("assign", [op], var) bb.insert_instruction(to_insert, index=i) - inst.operands[j] = var + if len(inst.operands) > j: + inst.operands[j] = var i += 1 i += 1 diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index aacb8d0d30..d03db373b7 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -436,13 +436,8 @@ 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 next_liveness: - # this branch seems unreachable (maybe due to make_ssa) - # %13/%14 is still live(!), so we make a copy of it - self.dup(assembly, stack, depth) - stack.poke(0, ret) - else: - stack.poke(depth, ret) + assert to_be_replaced not in next_liveness + stack.poke(depth, ret) return apply_line_numbers(inst, assembly) if opcode == "offset": From ead7c63b62d08431130a496b0646e960ac05c975 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 11 Jul 2025 14:27:18 +0200 Subject: [PATCH 27/47] fixes after merge --- vyper/venom/analysis/stack_order.py | 8 ++++---- vyper/venom/passes/function_inliner.py | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 7202afdac6..68c28fc83e 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -124,7 +124,7 @@ def _handle_bb(self, bb: IRBasicBlock) -> list[IROperand]: phi_positions: dict[IRVariable, int] = dict() for inst in bb.instructions: - if inst.opcode == "store": + if inst.opcode == "assign": inst_needed = self._handle_store(inst, stack) elif inst.opcode == "phi": inst_needed = self._handle_phi(inst, phis, phi_renames) @@ -188,7 +188,7 @@ def get_prefered_stack(self, origin: IRBasicBlock, succesors: list[IRBasicBlock] def _handle_store(self, inst: IRInstruction, stack: Stack) -> list[IROperand]: needed = [] - assert inst.opcode == "store" + assert inst.opcode == "assign" ops = inst.operands assert len(ops) == 1 op = ops[0] @@ -216,7 +216,7 @@ def _handle_store(self, inst: IRInstruction, stack: Stack) -> list[IROperand]: return needed def _handle_other_inst(self, inst: IRInstruction, stack: Stack) -> list[IROperand]: - assert inst.opcode not in ("store", "phi") + assert inst.opcode not in ("assign", "phi") bb = inst.parent if inst.is_bb_terminator: ops = [op for op in inst.operands if not isinstance(op, IRLabel)] @@ -266,7 +266,7 @@ def _handle_phi(self, inst: IRInstruction, phis: set[IRVariable], phi_renames) - # compute what will store do in bytecode def _handle_bb_store_types(self, bb: IRBasicBlock): for i, inst in enumerate(bb.instructions): - if inst.opcode != "store": + if inst.opcode != "assign": continue op = inst.operands[0] if isinstance(op, (IRLiteral, IRLabel)): diff --git a/vyper/venom/passes/function_inliner.py b/vyper/venom/passes/function_inliner.py index 90732b2848..618e1a9f2f 100644 --- a/vyper/venom/passes/function_inliner.py +++ b/vyper/venom/passes/function_inliner.py @@ -90,9 +90,8 @@ def _inline_function(self, func: IRFunction, call_sites: List[IRInstruction]) -> """ for call_site in call_sites: FloatAllocas(self.analyses_caches[func], func).run_pass() - return_bb = self._inline_call_site(func, call_site) + self._inline_call_site(func, call_site) fn = call_site.parent.parent - self._fix_phi(fn, call_site.parent, return_bb) self.analyses_caches[fn].invalidate_analysis(DFGAnalysis) self.analyses_caches[fn].invalidate_analysis(CFGAnalysis) From 54328dca22926c17beb1d4970ae091640bacebed Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 11 Jul 2025 14:59:48 +0200 Subject: [PATCH 28/47] msize could be reordered write after write --- 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 cceb6153cc..c6945f4dbf 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -138,7 +138,7 @@ def _calculate_dependency_graphs(self, bb: IRBasicBlock, out_stack: list[IROpera for read_inst in all_read_effects[write_effect]: self.eda[inst].add(read_inst) # prevent reordering write-after-write for the same effect - if write_effect in last_write_effects: + if (write_effect & ~effects.Effects.MSIZE) in last_write_effects: self.eda[inst].add(last_write_effects[write_effect]) last_write_effects[write_effect] = inst # clear previous read effects after a write From 62f3d85ef3f53777a0c57773d7516330ec236237 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 14 Jul 2025 10:31:22 +0200 Subject: [PATCH 29/47] flip order to simulate operands --- 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 d11e223c6f..c79a0e571b 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -36,7 +36,7 @@ def run_pass(self) -> None: order = self.stack_order.get_stack(bb) if bb in last_order and last_order[bb] == order: break - self.order = order + self.order = list(reversed(order)) self._process_basic_block(bb) From 724e2ae2af6f8a83beb3bd29d73b9f2c8f846da4 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 14 Jul 2025 18:51:12 +0200 Subject: [PATCH 30/47] fixes for outputs --- tests/unit/compiler/venom/test_stack_order.py | 403 ++++++++++++++++++ vyper/venom/analysis/stack_order.py | 35 +- vyper/venom/passes/dft.py | 12 +- 3 files changed, 435 insertions(+), 15 deletions(-) create mode 100644 tests/unit/compiler/venom/test_stack_order.py diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py new file mode 100644 index 0000000000..dcbfafcaf4 --- /dev/null +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -0,0 +1,403 @@ +import pytest + +from tests.venom_utils import PrePostChecker, parse_from_basic_block +from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.passes import AssignElimination, DFTPass, SimplifyCFGPass, SingleUseExpansion +from vyper.venom.venom_to_assembly import VenomCompiler + +# assing elim is there to have easier check +_check_pre_post = PrePostChecker([SingleUseExpansion, DFTPass, AssignElimination]) + + +def _check_no_change(pre): + _check_pre_post(pre, pre, hevm=False) + + +def test_stack_order_basic(): + pre = """ + main: + %1 = calldataload 1 + %2 = calldataload 2 + jmp @next + next: + %3 = add 1, %1 + %4 = add 1, %2 + return %4, %3 + """ + + post = """ + main: + %2 = calldataload 2 + %1 = calldataload 1 + jmp @next + next: + %3 = add 1, %1 + %4 = add 1, %2 + return %4, %3 + """ + + _check_pre_post(pre, post) + + ctx = parse_from_basic_block(post) + for fn in ctx.get_functions(): + ac = IRAnalysesCache(fn) + SingleUseExpansion(ac, fn).run_pass() + SimplifyCFGPass(ac, fn).run_pass() + + print(ctx) + + asm = VenomCompiler([ctx]).generate_evm() + print(asm) + assert asm == [ + "PUSH1", + 2, + "CALLDATALOAD", + "PUSH1", + 1, + "CALLDATALOAD", + "PUSH1", + 1, + "ADD", + "SWAP1", # swap out the result of the first add (only necessary swap) + "PUSH1", + 1, + "ADD", + "RETURN", + ] + + +def test_stack_order_basic2(): + pre = """ + main: + %1 = calldataload 1 + %2 = calldataload 2 + jmp @next + next: + %3 = add 1, %1 + %4 = add 1, %2 + return %3, %4 + """ + + post = """ + main: + %1 = calldataload 1 + %2 = calldataload 2 + jmp @next + next: + %4 = add 1, %2 + %3 = add 1, %1 + return %3, %4 + """ + + _check_pre_post(pre, post) + + ctx = parse_from_basic_block(post) + for fn in ctx.get_functions(): + ac = IRAnalysesCache(fn) + SingleUseExpansion(ac, fn).run_pass() + SimplifyCFGPass(ac, fn).run_pass() + + print(ctx) + + asm = VenomCompiler([ctx]).generate_evm() + print(asm) + assert asm == [ + "PUSH1", + 1, + "CALLDATALOAD", + "PUSH1", + 2, + "CALLDATALOAD", + "PUSH1", + 1, + "ADD", + "SWAP1", # swap out the result of the first add (only necessary swap) + "PUSH1", + 1, + "ADD", + "RETURN", + ] + + +def test_stack_order_split(): + pre = """ + main: + %1 = mload 1 + %2 = mload 2 + %3 = add 1, %2 + %cond = mload 3 + jnz %cond, @then, @else + then: + %4a = add 1, %1 + %5a = add 1, %3 + sink %5a, %4a + else: + %4b = add 1, %1 + %5b = add 1, %3 + sink %5b, %4b + """ + + post = """ + main: + %2 = mload 2 + %3 = add 1, %2 + %1 = mload 1 + %cond = mload 3 + jnz %cond, @then, @else + then: + %4a = add 1, %1 + %5a = add 1, %3 + sink %5a, %4a + else: + %4b = add 1, %1 + %5b = add 1, %3 + sink %5b, %4b + """ + + _check_pre_post(pre, post) + + +def test_stack_order_split2(): + pre = """ + main: + %1 = mload 1 + %2 = mload 2 + %3 = add 1, %2 + %cond = mload 3 + jnz %cond, @then, @else + then: + %4a = add 1, %1 + %5a = add 1, %2 + sink %5a, %4a + else: + %4b = add 1, %1 + %5b = add 1, %3 + sink %5b, %4b + """ + + post = """ + main: + %2 = mload 2 + %3 = add 1, %2 + %1 = mload 1 + %cond = mload 3 + jnz %cond, @then, @else + then: + %4a = add 1, %1 + %5a = add 1, %2 + sink %5a, %4a + else: + %4b = add 1, %1 + %5b = add 1, %3 + sink %5b, %4b + """ + + _check_pre_post(pre, post) + + +def test_stack_order_join(): + pre = """ + main: + %cond = param + %1 = mload 1 + %2 = mload 2 + jnz %cond, @then, @else + then: + %3a = mload 3 + mstore 1000, %3a + jmp @join + else: + %3b = mload 3 + mstore 2000, %3b + jmp @join + join: + sink %1 + """ + + post = """ + main: + %cond = param + %2 = mload 2 + %1 = mload 1 + jnz %cond, @then, @else + then: + %3a = mload 3 + mstore 1000, %3a + jmp @join + else: + %3b = mload 3 + mstore 2000, %3b + jmp @join + join: + sink %1 + """ + + _check_pre_post(pre, post) + + +def test_stack_order_join_unmergable_stacks(): + pre = """ + main: + %cond = param + %1 = mload 1 + %2 = mload 2 + jnz %cond, @then, @else + then: + mstore 1000, %1 + jmp @join + else: + mstore 2000, %2 + jmp @join + join: + sink %1 + """ + + _check_no_change(pre) + + +def test_stack_order_phi(): + pre = """ + main: + %par = param + jnz %par, @then, @else + then: + %1a = add 1, %par + %2a = mload 10 + mstore 1000, %2a + jmp @join + else: + %1b = add 2, %par + %2b = mload 10 + mstore 1000, %2b + jmp @join + join: + %1 = phi @then, %1a, @else, %1b + %res = add 1, %1 ; properly use the value + sink %res + """ + + post = """ + main: + %par = param + jnz %par, @then, @else + then: + %2a = mload 10 + mstore 1000, %2a + %1a = add 1, %par + jmp @join + else: + %2b = mload 10 + mstore 1000, %2b + %1b = add 2, %par + jmp @join + join: + %1 = phi @then, %1a, @else, %1b + %res = add 1, %1 ; properly use the value + sink %res + """ + + _check_pre_post(pre, post) + + +# TODO: fix this xfail before merge +#@pytest.mark.xfail +def test_stack_order_more_phi(): + pre = """ + main: + %par = param + jnz %par, @then, @else + then: + %1a = add 1, %par + %2a = add 2, %par + jmp @join + else: + %1b = add 3, %par + %2b = add 4, %par + jmp @join + join: + %2 = phi @then, %2a, @else, %2b + %1 = phi @then, %1a, @else, %1b + %res1 = add 1, %1 ; properly use the value + %res2 = add 1, %2 ; properly use the value + sink %res2, %res1 + """ + + post = """ + main: + %par = param + jnz %par, @then, @else + then: + %2a = add 2, %par + %1a = add 1, %par + jmp @join + else: + %2b = add 4, %par + %1b = add 3, %par + jmp @join + join: + %2 = phi @then, %2a, @else, %2b + %1 = phi @then, %1a, @else, %1b + %res1 = add 1, %1 ; properly use the value + %res2 = add 1, %2 ; properly use the value + sink %res2, %res1 + """ + + _check_pre_post(pre, post) + +def test_stack_order_entry_instruction(): + pre = """ + main: + %p = param + %1 = add %p, 1 + assert %1 + %2 = add %p, 2 + %cond = iszero %p + jnz %cond, @then, @else + then: + %3a = mload 0 + mstore 100, %3a + jmp @join + else: + %3b = mload 0 + mstore 100, %3b + jmp @join + join: + sink %1, %2 + """ + + post = """ + main: + %p = param + %2 = add %p, 2 + %cond = iszero %p + %1 = add %p, 1 + assert %1 + jnz %cond, @then, @else + then: + %3a = mload 0 + mstore 100, %3a + jmp @join + else: + %3b = mload 0 + mstore 100, %3b + jmp @join + join: + sink %1, %2 + """ + + _check_pre_post(pre, post) + +def test_stack_order_two_trees(): + pre = """ + main: + %1 = param + %2 = param + %cond = iszero %2 + assert %cond + %3 = 3 + jmp @after + after: + sink %3, %2, %1 + """ + + _check_no_change(pre) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index c7bcfdaeb2..fa8f7320d3 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -4,7 +4,7 @@ from vyper.venom.analysis import LivenessAnalysis, CFGAnalysis # needed [top, ... , bottom] -type Needed = list[IROperand] +type Needed = list[IRVariable] type Stack = list[IROperand] @@ -28,7 +28,7 @@ def _swap_to(stack: Stack, depth: int): def _max_same_prefix(stack_a: Needed, stack_b: Needed): - res: list[IROperand] = [] + res: Needed = [] for a, b in zip(stack_a, stack_b): if a != b: break @@ -50,27 +50,37 @@ def __init__(self, function: IRFunction, ac: IRAnalysesCache): def analyze_bb(self, bb: IRBasicBlock) -> Needed: + #breakpoint() self.needed = [] self.stack = [] for inst in bb.instructions: if inst.opcode == "assign": self._handle_assign(inst) - if inst.opcode == "phi": + elif inst.opcode == "phi": self._handle_inst(inst) elif inst.is_bb_terminator: self._handle_terminator(inst) else: self._handle_inst(inst) + if len(inst.operands) > 0: + if not inst.is_bb_terminator: + assert self.stack[-len(inst.operands):] == inst.operands, (inst, self.stack, inst.operands) + self.stack = self.stack[:-len(inst.operands)] + if inst.output is not None: + self.stack.append(inst.output) + for pred in self.cfg.cfg_in(bb): - self._from_to[(pred, bb)] = self.needed + self._from_to[(pred, bb)] = self.needed.copy() return self.needed def get_stack(self, bb: IRBasicBlock) -> Needed: succs = self.cfg.cfg_out(bb) - orders = [self.from_to(bb, succ) for succ in succs] + for succ in succs: + self.analyze_bb(succ) + orders = [self._from_to.get((bb, succ), []) for succ in succs] return self._merge(orders) def from_to(self, origin: IRBasicBlock, successor: IRBasicBlock) -> Needed: @@ -84,8 +94,8 @@ def from_to(self, origin: IRBasicBlock, successor: IRBasicBlock) -> Needed: def _handle_assign(self, inst: IRInstruction): assert inst.opcode == "assign" + assert inst.output is not None - self.liveness.live_vars_at(inst) index = inst.parent.instructions.index(inst) next_inst = inst.parent.instructions[index + 1] next_live = self.liveness.live_vars_at(next_inst) @@ -105,7 +115,7 @@ def _handle_assign(self, inst: IRInstruction): else: _swap(self.stack, src) - def _add_needed(self, op: IROperand): + def _add_needed(self, op: IRVariable): if op not in self.needed: self.needed.append(op) @@ -116,7 +126,6 @@ def _reorder(self, target_stack: Stack): depth = count - index - 1 _swap(self.stack, op) _swap_to(self.stack, depth) - self._add_needed(op) if len(target_stack) != 0: assert target_stack == self.stack[-len(target_stack):], (target_stack, self.stack) @@ -126,7 +135,7 @@ def _reorder(self, target_stack: Stack): def _handle_inst(self, inst: IRInstruction): ops = inst.operands for op in ops: - if isinstance(op, IRVariable): + if isinstance(op, IRVariable) and op not in self.stack: self._add_needed(op) if op not in self.stack: self.stack.append(op) @@ -142,10 +151,12 @@ def _merge(self, orders: list[Needed]) -> Needed: def _handle_terminator(self, inst: IRInstruction): bb = inst.parent - orders = [self.from_to(bb, succ) for succ in self.cfg.cfg_out(bb)] + orders = [self._from_to.get((bb, succ), []) for succ in self.cfg.cfg_out(bb)] ops = (op for op in inst.operands if isinstance(op, IRVariable)) for op in ops: - self._add_needed(op) + if op not in self.stack: + self._add_needed(op) for op in self._merge(orders): - self._add_needed(op) + if op not in self.stack: + self._add_needed(op) diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index c79a0e571b..2daa7fe400 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -4,7 +4,7 @@ from vyper.utils import OrderedSet from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, CFGAnalysis from vyper.venom.analysis.stack_order import StackOrderAnalysis -from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IROperand +from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRVariable from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass @@ -28,7 +28,7 @@ def run_pass(self) -> None: worklist = deque(self.cfg.dfs_post_walk) - last_order: dict[IRBasicBlock, list[IROperand]] = dict() + last_order: dict[IRBasicBlock, list[IRVariable]] = dict() while len(worklist) > 0: bb = worklist.popleft() @@ -36,13 +36,19 @@ def run_pass(self) -> None: order = self.stack_order.get_stack(bb) if bb in last_order and last_order[bb] == order: break + last_order[bb] = order self.order = list(reversed(order)) + #self.order = order self._process_basic_block(bb) + + for pred in self.cfg.cfg_in(bb): + worklist.append(pred) self.analyses_cache.invalidate_analysis(LivenessAnalysis) def _process_basic_block(self, bb: IRBasicBlock) -> None: + #breakpoint() self._calculate_dependency_graphs(bb) self.instructions = list(bb.pseudo_instructions) non_phi_instructions = list(bb.non_phi_instructions) @@ -135,7 +141,7 @@ def _calculate_dependency_graphs(self, bb: IRBasicBlock) -> None: for read_inst in all_read_effects[write_effect]: self.eda[inst].add(read_inst) # prevent reordering write-after-write for the same effect - if write_effect in last_write_effects: + if (write_effect & ~effects.Effects.MSIZE) in last_write_effects: self.eda[inst].add(last_write_effects[write_effect]) last_write_effects[write_effect] = inst # clear previous read effects after a write From 8c43920cafeec1428e597ca82c2a5624887c61f4 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 12 Aug 2025 13:48:08 +0200 Subject: [PATCH 31/47] removed stray breakpoint --- vyper/venom/analysis/stack_order.py | 1 - 1 file changed, 1 deletion(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index fa8f7320d3..1ebb3e80db 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -50,7 +50,6 @@ def __init__(self, function: IRFunction, ac: IRAnalysesCache): def analyze_bb(self, bb: IRBasicBlock) -> Needed: - #breakpoint() self.needed = [] self.stack = [] From 74b800fe28a854eb229b15bfb2b78d218a48ccc2 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 12 Aug 2025 15:37:06 +0200 Subject: [PATCH 32/47] fixes after merge --- vyper/venom/analysis/__init__.py | 2 +- vyper/venom/passes/dft.py | 12 +++++----- vyper/venom/passes/single_use_expansion.py | 12 ---------- vyper/venom/venom_to_assembly.py | 28 +++------------------- 4 files changed, 10 insertions(+), 44 deletions(-) diff --git a/vyper/venom/analysis/__init__.py b/vyper/venom/analysis/__init__.py index 978fbdeb76..4694b64cf4 100644 --- a/vyper/venom/analysis/__init__.py +++ b/vyper/venom/analysis/__init__.py @@ -7,5 +7,5 @@ from .mem_alias import MemoryAliasAnalysis from .mem_ssa import MemSSA from .reachable import ReachableAnalysis -from .stack_order import StackOrder +from .stack_order import StackOrderAnalysis from .var_definition import VarDefinition diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index 30d07ffc53..66abf6e854 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -18,13 +18,13 @@ class DFTPass(IRPass): # "effect dependency analysis" eda: dict[IRInstruction, OrderedSet[IRInstruction]] - stack_order: StackOrder + stack_order: StackOrderAnalysis cfg: CFGAnalysis def run_pass(self) -> None: self.data_offspring = {} self.visited_instructions: OrderedSet[IRInstruction] = OrderedSet() - self.stack_order = StackOrder(self.analyses_cache, self.function) + self.stack_order = StackOrderAnalysis(self.function, self.analyses_cache) self.cfg = self.analyses_cache.request_analysis(CFGAnalysis) self.dfg = self.analyses_cache.force_analysis(DFGAnalysis) @@ -72,12 +72,12 @@ def _process_basic_block(self, bb: IRBasicBlock) -> None: self.visited_instructions = OrderedSet() for inst in entry_instructions_list: - self._process_instruction_r(self.instructions, inst, stack_order) + self._process_instruction_r(self.instructions, inst) bb.instructions = self.instructions assert bb.is_terminated, f"Basic block should be terminated {bb}" def _process_instruction_r( - self, instructions: list[IRInstruction], inst: IRInstruction, stack_order: list[IROperand] + self, instructions: list[IRInstruction], inst: IRInstruction ): if inst in self.visited_instructions: return @@ -110,11 +110,11 @@ def cost(x: IRInstruction) -> int | float: inst.flip() for dep_inst in children: - self._process_instruction_r(instructions, dep_inst, stack_order) + self._process_instruction_r(instructions, dep_inst) instructions.append(inst) - def _calculate_dependency_graphs(self, bb: IRBasicBlock, out_stack: list[IROperand]) -> None: + def _calculate_dependency_graphs(self, bb: IRBasicBlock) -> None: # ida: instruction dependency analysis self.dda = defaultdict(OrderedSet) self.eda = defaultdict(OrderedSet) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 6ea66e7c5e..6e8d2a2aa8 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -47,18 +47,6 @@ def _process_bb(self, bb): uses = self.dfg.get_uses(op) if len(uses) == 1 and len([x for x in inst.operands if x == op]) == 1: continue - var = self.function.get_next_variable() - to_insert = IRInstruction("assign", [op], var) - bb.insert_instruction(to_insert, index=i) - inst.operands[j] = var - i += 1 - - if isinstance(op, IRLiteral): - var = self.function.get_next_variable() - to_insert = IRInstruction("assign", [op], var) - bb.insert_instruction(to_insert, index=i) - inst.operands[j] = var - i += 1 if not isinstance(op, (IRLiteral, IRVariable)): # IRLabels are special in certain instructions (e.g., jmp) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 153d1e62ed..0c1e5dbeb3 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -23,7 +23,7 @@ ) from vyper.venom.context import IRContext, IRFunction from vyper.venom.stack_model import StackModel -from vyper.venom.analysis import StackOrder +from vyper.venom.analysis import StackOrderAnalysis DEBUG_SHOW_COST = False if DEBUG_SHOW_COST: @@ -160,8 +160,6 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: self.liveness = ac.request_analysis(LivenessAnalysis) self.dfg = ac.request_analysis(DFGAnalysis) self.cfg = ac.request_analysis(CFGAnalysis) - self.stack_order = StackOrder(ac, fn) - self.stack_order.calculate_all_orders() assert self.cfg.is_normalized(), "Non-normalized CFG!" @@ -466,28 +464,8 @@ def _generate_evm_for_instruction( # guaranteed by cfg normalization+simplification assert len(self.cfg.cfg_in(next_bb)) > 1 - liv = self.liveness.input_vars_from(inst.parent, next_bb) - target_stack = list(reversed(self.stack_order.from_to_stack[(inst.parent, next_bb)])) - # NOTE: in general the stack can contain multiple copies of - # the same variable, however, before a jump that is not possible - import sys - #print(target_stack, list(liv - tmp = False - if target_stack != list(liv): - print(inst.parent, target_stack, list(liv), file=sys.stderr) - tmp = True - #for op in reversed(liv): - #if op not in target_stack: - #target_stack.insert(0, op) - - target_stack = list(self.liveness.input_vars_from(inst.parent, next_bb)) - - - if tmp: - print(stack, file=sys.stderr) - self._stack_reorder(assembly, stack, target_stack) - if tmp: - print(stack, assembly, file=sys.stderr) + target_stack = self.liveness.input_vars_from(inst.parent, next_bb) + self._stack_reorder(assembly, stack, list(target_stack)) if inst.is_commutative: cost_no_swap = self._stack_reorder([], stack, operands, dry_run=True) From a6ff90a975fb57e06cef4d566276f7c2db1c342a Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 12 Aug 2025 16:37:08 +0200 Subject: [PATCH 33/47] revert the phi precondition --- vyper/venom/venom_to_assembly.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 0c1e5dbeb3..c8641b3c88 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -437,8 +437,13 @@ 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) - assert to_be_replaced not in next_liveness - stack.poke(depth, ret) + if to_be_replaced in next_liveness: + # this branch seems unreachable (maybe due to make_ssa) + # %13/%14 is still live(!), so we make a copy of it + self.dup(assembly, stack, depth) + stack.poke(0, ret) + else: + stack.poke(depth, ret) return apply_line_numbers(inst, assembly) if opcode == "offset": From c73cb15c172925eee79fc8a6461cd1f4ea06fbde Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 12 Aug 2025 16:48:38 +0200 Subject: [PATCH 34/47] added xfail --- tests/unit/compiler/venom/test_stack_order.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index 65d4052214..2dd7180b83 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -300,7 +300,7 @@ def test_stack_order_phi(): # TODO: fix this xfail before merge -#@pytest.mark.xfail +@pytest.mark.xfail def test_stack_order_more_phi(): pre = """ main: @@ -345,6 +345,7 @@ def test_stack_order_more_phi(): _check_pre_post(pre, post) +@pytest.mark.xfail def test_stack_order_entry_instruction(): pre = """ main: @@ -388,6 +389,7 @@ def test_stack_order_entry_instruction(): _check_pre_post(pre, post) +@pytest.mark.xfail def test_stack_order_two_trees(): pre = """ main: From 037920c1917576df0dd0113bcaae1eb2034a2440 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 12 Aug 2025 16:58:02 +0200 Subject: [PATCH 35/47] removed `type` since it is not in 3.11 --- vyper/venom/analysis/stack_order.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index e2afe34c8d..98ae6a00d0 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -4,9 +4,9 @@ from vyper.venom.analysis import LivenessAnalysis, CFGAnalysis # needed [top, ... , bottom] -type Needed = list[IRVariable] +Needed = list[IRVariable] -type Stack = list[IROperand] +Stack = list[IROperand] def _swap(stack: Stack, op: IROperand): top = len(stack) - 1 From f8b5205b2a93747c2fc44825bc515797285efa9c Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 12 Aug 2025 17:11:08 +0200 Subject: [PATCH 36/47] lint --- tests/unit/compiler/venom/test_stack_order.py | 3 +- vyper/venom/analysis/stack_order.py | 32 +++++++++---------- vyper/venom/passes/dft.py | 17 ++++------ vyper/venom/passes/function_inliner.py | 1 - vyper/venom/venom_to_assembly.py | 5 +-- 5 files changed, 28 insertions(+), 30 deletions(-) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index 2dd7180b83..70bcc1b046 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -386,9 +386,10 @@ def test_stack_order_entry_instruction(): join: sink %1, %2 """ - + _check_pre_post(pre, post) + @pytest.mark.xfail def test_stack_order_two_trees(): pre = """ diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 98ae6a00d0..0eccc984ff 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -1,13 +1,14 @@ -from vyper.venom.basicblock import IROperand, IRBasicBlock, IRInstruction, IRVariable -from vyper.venom.function import IRFunction +from vyper.venom.analysis import CFGAnalysis, LivenessAnalysis from vyper.venom.analysis.analysis import IRAnalysesCache -from vyper.venom.analysis import LivenessAnalysis, CFGAnalysis +from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IROperand, IRVariable +from vyper.venom.function import IRFunction # needed [top, ... , bottom] Needed = list[IRVariable] Stack = list[IROperand] + def _swap(stack: Stack, op: IROperand): top = len(stack) - 1 index = None @@ -35,23 +36,22 @@ def _max_same_prefix(stack_a: Needed, stack_b: Needed): res.append(a) return res + class StackOrderAnalysis: function: IRFunction liveness: LivenessAnalysis cfg: CFGAnalysis _from_to: dict[tuple[IRBasicBlock, IRBasicBlock], Needed] - def __init__(self, function: IRFunction, ac: IRAnalysesCache): self._from_to = dict() self.ac = ac self.liveness = ac.request_analysis(LivenessAnalysis) self.cfg = ac.request_analysis(CFGAnalysis) - def analyze_bb(self, bb: IRBasicBlock) -> Needed: - self.needed = [] - self.stack = [] + self.needed: Needed = [] + self.stack: Stack = [] for inst in bb.instructions: if inst.opcode == "assign": @@ -65,8 +65,12 @@ def analyze_bb(self, bb: IRBasicBlock) -> Needed: if len(inst.operands) > 0: if not inst.is_bb_terminator: - assert self.stack[-len(inst.operands):] == inst.operands, (inst, self.stack, inst.operands) - self.stack = self.stack[:-len(inst.operands)] + assert self.stack[-len(inst.operands) :] == inst.operands, ( + inst, + self.stack, + inst.operands, + ) + self.stack = self.stack[: -len(inst.operands)] if inst.output is not None: self.stack.append(inst.output) @@ -74,7 +78,7 @@ def analyze_bb(self, bb: IRBasicBlock) -> Needed: self._from_to[(pred, bb)] = self.needed.copy() return self.needed - + def get_stack(self, bb: IRBasicBlock) -> Needed: succs = self.cfg.cfg_out(bb) for succ in succs: @@ -88,7 +92,7 @@ def from_to(self, origin: IRBasicBlock, successor: IRBasicBlock) -> Needed: for var in self.liveness.input_vars_from(origin, successor): if var not in target: target.append(var) - + return target def _handle_assign(self, inst: IRInstruction): @@ -127,9 +131,7 @@ def _reorder(self, target_stack: Stack): _swap_to(self.stack, depth) if len(target_stack) != 0: - assert target_stack == self.stack[-len(target_stack):], (target_stack, self.stack) - - + assert target_stack == self.stack[-len(target_stack) :], (target_stack, self.stack) def _handle_inst(self, inst: IRInstruction): ops = inst.operands @@ -158,5 +160,3 @@ def _handle_terminator(self, inst: IRInstruction): for op in self._merge(orders): if op not in self.stack: self._add_needed(op) - - diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index 66abf6e854..a985e1f7d7 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -2,7 +2,7 @@ import vyper.venom.effects as effects from vyper.utils import OrderedSet -from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, CFGAnalysis +from vyper.venom.analysis import CFGAnalysis, DFGAnalysis, LivenessAnalysis from vyper.venom.analysis.stack_order import StackOrderAnalysis from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRVariable from vyper.venom.function import IRFunction @@ -43,17 +43,16 @@ def run_pass(self) -> None: break last_order[bb] = order self.order = list(reversed(order)) - #self.order = order + # self.order = order self._process_basic_block(bb) - + for pred in self.cfg.cfg_in(bb): worklist.append(pred) - self.analyses_cache.invalidate_analysis(LivenessAnalysis) def _process_basic_block(self, bb: IRBasicBlock) -> None: - #breakpoint() + # breakpoint() self._calculate_dependency_graphs(bb) self.instructions = list(bb.pseudo_instructions) non_phi_instructions = list(bb.non_phi_instructions) @@ -76,9 +75,7 @@ def _process_basic_block(self, bb: IRBasicBlock) -> None: bb.instructions = self.instructions assert bb.is_terminated, f"Basic block should be terminated {bb}" - def _process_instruction_r( - self, instructions: list[IRInstruction], inst: IRInstruction - ): + def _process_instruction_r(self, instructions: list[IRInstruction], inst: IRInstruction): if inst in self.visited_instructions: return self.visited_instructions.add(inst) @@ -129,8 +126,8 @@ def _calculate_dependency_graphs(self, bb: IRBasicBlock) -> None: for inst in non_phis: if inst.is_bb_terminator: - for op in self.order: - dep = self.dfg.get_producing_instruction(op) + for var in self.order: + dep = self.dfg.get_producing_instruction(var) if dep is not None and dep.parent == bb: self.dda[inst].add(dep) for op in inst.operands: diff --git a/vyper/venom/passes/function_inliner.py b/vyper/venom/passes/function_inliner.py index 618e1a9f2f..6d6f355d2b 100644 --- a/vyper/venom/passes/function_inliner.py +++ b/vyper/venom/passes/function_inliner.py @@ -144,7 +144,6 @@ def _inline_function(self, func: IRFunction, call_sites: List[IRInstruction]) -> # demote to alloca so that mem2var will work inst.opcode = "alloca" - def _inline_call_site(self, func: IRFunction, call_site: IRInstruction): """ Inline function into call site. diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index dc1561834b..22aad9e756 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -27,7 +27,6 @@ ) from vyper.venom.context import IRContext, IRFunction from vyper.venom.stack_model import StackModel -from vyper.venom.analysis import StackOrderAnalysis DEBUG_SHOW_COST = False if DEBUG_SHOW_COST: @@ -227,7 +226,9 @@ def _stack_reorder( if len(stack_ops) == 0: return 0 - assert len(stack_ops) == len(set(stack_ops)), f"duplicated stack {stack_ops}" # precondition + assert len(stack_ops) == len( + set(stack_ops) + ), f"duplicated stack {stack_ops}" # precondition cost = 0 for i, op in enumerate(stack_ops): From 76843835e6d535bb380aef2689b2f3cde3d72310 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 14 Aug 2025 12:59:26 +0300 Subject: [PATCH 37/47] comment on ordering intuition --- vyper/venom/passes/dft.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index a985e1f7d7..ec90f6e3bb 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -86,6 +86,12 @@ def _process_instruction_r(self, instructions: list[IRInstruction], inst: IRInst children = list(self.dda[inst] | self.eda[inst]) def cost(x: IRInstruction) -> int | float: + # intuition: + # effect-only dependencies which have data dependencies + # effect-only dependencies which have no data dependencies + # indirect data dependencies (offspring of operands) + # direct data dependencies (order of operands) + if (x not in self.dda[inst] and x in self.eda[inst]) or inst.flippable: ret = -1 * int(len(self.data_offspring[x]) > 0) elif x.output in inst.operands: From 33b4f954f97f128a3ac0aecb4aeca182d73dab65 Mon Sep 17 00:00:00 2001 From: Hodan Date: Thu, 14 Aug 2025 11:45:10 +0200 Subject: [PATCH 38/47] disable optimistic swap before terminators --- vyper/venom/venom_to_assembly.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 22aad9e756..4212d3fa17 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -623,6 +623,12 @@ def _optimistic_swap(self, assembly, inst, next_liveness, stack): if DEBUG_SHOW_COST: stack0 = stack.copy() + next_index = inst.parent.instructions.index(inst) + next_inst = inst.parent.instructions[next_index + 1] + + if next_inst.is_bb_terminator: + return + next_scheduled = next_liveness.last() cost = 0 if not self.dfg.are_equivalent(inst.output, next_scheduled): From 656093b5feaa28626b5f4ecd1317322e904d03aa Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 15 Aug 2025 15:04:53 +0200 Subject: [PATCH 39/47] removed liveness calculation from single use pass --- vyper/venom/passes/single_use_expansion.py | 1 - 1 file changed, 1 deletion(-) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 6e8d2a2aa8..de911a35cf 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -21,7 +21,6 @@ class SingleUseExpansion(IRPass): def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) - self.liveness = self.analyses_cache.request_analysis(LivenessAnalysis) for bb in self.function.get_basic_blocks(): self._process_bb(bb) From 31424cea4dd6254004717161e5266006bb6f78cf Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 15 Aug 2025 15:22:21 +0200 Subject: [PATCH 40/47] cleanup --- vyper/venom/analysis/stack_order.py | 2 +- vyper/venom/passes/dft.py | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 0eccc984ff..6739ffed1d 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -43,7 +43,7 @@ class StackOrderAnalysis: cfg: CFGAnalysis _from_to: dict[tuple[IRBasicBlock, IRBasicBlock], Needed] - def __init__(self, function: IRFunction, ac: IRAnalysesCache): + def __init__(self, ac: IRAnalysesCache): self._from_to = dict() self.ac = ac self.liveness = ac.request_analysis(LivenessAnalysis) diff --git a/vyper/venom/passes/dft.py b/vyper/venom/passes/dft.py index ec90f6e3bb..175d6010cf 100644 --- a/vyper/venom/passes/dft.py +++ b/vyper/venom/passes/dft.py @@ -24,12 +24,10 @@ class DFTPass(IRPass): def run_pass(self) -> None: self.data_offspring = {} self.visited_instructions: OrderedSet[IRInstruction] = OrderedSet() - self.stack_order = StackOrderAnalysis(self.function, self.analyses_cache) - self.cfg = self.analyses_cache.request_analysis(CFGAnalysis) self.dfg = self.analyses_cache.force_analysis(DFGAnalysis) self.cfg = self.analyses_cache.request_analysis(CFGAnalysis) - self.stack_order = StackOrderAnalysis(self.function, self.analyses_cache) + self.stack_order = StackOrderAnalysis(self.analyses_cache) worklist = deque(self.cfg.dfs_post_walk) @@ -43,7 +41,6 @@ def run_pass(self) -> None: break last_order[bb] = order self.order = list(reversed(order)) - # self.order = order self._process_basic_block(bb) for pred in self.cfg.cfg_in(bb): @@ -52,7 +49,6 @@ def run_pass(self) -> None: self.analyses_cache.invalidate_analysis(LivenessAnalysis) def _process_basic_block(self, bb: IRBasicBlock) -> None: - # breakpoint() self._calculate_dependency_graphs(bb) self.instructions = list(bb.pseudo_instructions) non_phi_instructions = list(bb.non_phi_instructions) From 9b59d06389b1f97b0e73d8edc7115eb40bb3414c Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 22 Aug 2025 13:36:48 +0200 Subject: [PATCH 41/47] phi handle --- vyper/venom/analysis/stack_order.py | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 6739ffed1d..43645287d6 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -1,6 +1,6 @@ from vyper.venom.analysis import CFGAnalysis, LivenessAnalysis from vyper.venom.analysis.analysis import IRAnalysesCache -from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IROperand, IRVariable +from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IROperand, IRVariable, IRLabel from vyper.venom.function import IRFunction # needed [top, ... , bottom] @@ -52,6 +52,7 @@ def __init__(self, ac: IRAnalysesCache): def analyze_bb(self, bb: IRBasicBlock) -> Needed: self.needed: Needed = [] self.stack: Stack = [] + self.translates: dict[IRVariable, dict[IRLabel, IRVariable]] = dict() for inst in bb.instructions: if inst.opcode == "assign": @@ -75,8 +76,14 @@ def analyze_bb(self, bb: IRBasicBlock) -> Needed: self.stack.append(inst.output) for pred in self.cfg.cfg_in(bb): - self._from_to[(pred, bb)] = self.needed.copy() - + translates: Needed = [] + for var in self.needed: + if var in self.translates: + translates.append(self.translates[var][pred]) + else: + translates.append(var) + self._from_to[(pred, bb)] = translates + return self.needed def get_stack(self, bb: IRBasicBlock) -> Needed: @@ -160,3 +167,14 @@ def _handle_terminator(self, inst: IRInstruction): for op in self._merge(orders): if op not in self.stack: self._add_needed(op) + + def _handle_phi(self, inst: IRInstruction): + assert inst.opcode == "phi" + assert inst.output is not None + + out_var = inst.output + self.translates[out_var] = dict() + + for label, var in inst.phi_operands: + assert isinstance(var, IRVariable) + self.translates[out_var][label] = var From 86b7940ce02e048911338b9d97e5d70d0e4201d5 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 22 Aug 2025 14:11:18 +0200 Subject: [PATCH 42/47] called it --- vyper/venom/analysis/stack_order.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 43645287d6..400a3ffe2d 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -58,7 +58,8 @@ def analyze_bb(self, bb: IRBasicBlock) -> Needed: if inst.opcode == "assign": self._handle_assign(inst) elif inst.opcode == "phi": - self._handle_inst(inst) + self._handle_phi(inst) + continue elif inst.is_bb_terminator: self._handle_terminator(inst) else: @@ -79,7 +80,7 @@ def analyze_bb(self, bb: IRBasicBlock) -> Needed: translates: Needed = [] for var in self.needed: if var in self.translates: - translates.append(self.translates[var][pred]) + translates.append(self.translates[var][pred.label]) else: translates.append(var) self._from_to[(pred, bb)] = translates From eab2fe09abf0e0c8523f777bbbd0bf8b10275b99 Mon Sep 17 00:00:00 2001 From: Hodan Date: Wed, 3 Sep 2025 16:39:14 +0200 Subject: [PATCH 43/47] changes to passes - removed unnecesary condition --- vyper/venom/analysis/stack_order.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 400a3ffe2d..6e6e8b4667 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -1,6 +1,6 @@ from vyper.venom.analysis import CFGAnalysis, LivenessAnalysis from vyper.venom.analysis.analysis import IRAnalysesCache -from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IROperand, IRVariable, IRLabel +from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRLabel, IROperand, IRVariable from vyper.venom.function import IRFunction # needed [top, ... , bottom] @@ -84,7 +84,7 @@ def analyze_bb(self, bb: IRBasicBlock) -> Needed: else: translates.append(var) self._from_to[(pred, bb)] = translates - + return self.needed def get_stack(self, bb: IRBasicBlock) -> Needed: @@ -117,8 +117,8 @@ def _handle_assign(self, inst: IRInstruction): self.stack.append(src) elif src in next_live: self.stack.append(src) - if src in self.stack: - self._add_needed(src) + assert src in self.stack + self._add_needed(src) else: if src not in self.stack: self.stack.append(src) @@ -175,7 +175,7 @@ def _handle_phi(self, inst: IRInstruction): out_var = inst.output self.translates[out_var] = dict() - + for label, var in inst.phi_operands: assert isinstance(var, IRVariable) self.translates[out_var][label] = var From cef210627d02c01a4b9b41395b15cf9ee7b5c896 Mon Sep 17 00:00:00 2001 From: Hodan Date: Wed, 3 Sep 2025 16:42:57 +0200 Subject: [PATCH 44/47] fixed test and removed xfail --- tests/unit/compiler/venom/test_stack_order.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index 70bcc1b046..38765b0a56 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -46,7 +46,7 @@ def test_stack_order_basic(): print(ctx) - asm = VenomCompiler([ctx]).generate_evm() + asm = VenomCompiler(ctx).generate_evm_assembly() print(asm) assert asm == [ "PUSH1", @@ -99,7 +99,7 @@ def test_stack_order_basic2(): print(ctx) - asm = VenomCompiler([ctx]).generate_evm() + asm = VenomCompiler(ctx).generate_evm_assembly() print(asm) assert asm == [ "PUSH1", @@ -299,8 +299,6 @@ def test_stack_order_phi(): _check_pre_post(pre, post) -# TODO: fix this xfail before merge -@pytest.mark.xfail def test_stack_order_more_phi(): pre = """ main: From 1ad33b72ba8b5b870fee65bc1af40f235a8d248e Mon Sep 17 00:00:00 2001 From: Hodan Date: Wed, 3 Sep 2025 16:58:28 +0200 Subject: [PATCH 45/47] Revert "called it" This reverts commit 86b7940ce02e048911338b9d97e5d70d0e4201d5. --- vyper/venom/analysis/stack_order.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 6e6e8b4667..02e8da8390 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -58,8 +58,7 @@ def analyze_bb(self, bb: IRBasicBlock) -> Needed: if inst.opcode == "assign": self._handle_assign(inst) elif inst.opcode == "phi": - self._handle_phi(inst) - continue + self._handle_inst(inst) elif inst.is_bb_terminator: self._handle_terminator(inst) else: @@ -80,7 +79,7 @@ def analyze_bb(self, bb: IRBasicBlock) -> Needed: translates: Needed = [] for var in self.needed: if var in self.translates: - translates.append(self.translates[var][pred.label]) + translates.append(self.translates[var][pred]) else: translates.append(var) self._from_to[(pred, bb)] = translates From 48a56cbe02df70350655a539019592df5d2d0200 Mon Sep 17 00:00:00 2001 From: Hodan Date: Wed, 3 Sep 2025 17:01:41 +0200 Subject: [PATCH 46/47] Revert "phi handle" This reverts commit 9b59d06389b1f97b0e73d8edc7115eb40bb3414c. --- vyper/venom/analysis/stack_order.py | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/vyper/venom/analysis/stack_order.py b/vyper/venom/analysis/stack_order.py index 02e8da8390..01c671ba72 100644 --- a/vyper/venom/analysis/stack_order.py +++ b/vyper/venom/analysis/stack_order.py @@ -1,6 +1,6 @@ from vyper.venom.analysis import CFGAnalysis, LivenessAnalysis from vyper.venom.analysis.analysis import IRAnalysesCache -from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRLabel, IROperand, IRVariable +from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IROperand, IRVariable from vyper.venom.function import IRFunction # needed [top, ... , bottom] @@ -52,7 +52,6 @@ def __init__(self, ac: IRAnalysesCache): def analyze_bb(self, bb: IRBasicBlock) -> Needed: self.needed: Needed = [] self.stack: Stack = [] - self.translates: dict[IRVariable, dict[IRLabel, IRVariable]] = dict() for inst in bb.instructions: if inst.opcode == "assign": @@ -76,13 +75,7 @@ def analyze_bb(self, bb: IRBasicBlock) -> Needed: self.stack.append(inst.output) for pred in self.cfg.cfg_in(bb): - translates: Needed = [] - for var in self.needed: - if var in self.translates: - translates.append(self.translates[var][pred]) - else: - translates.append(var) - self._from_to[(pred, bb)] = translates + self._from_to[(pred, bb)] = self.needed.copy() return self.needed @@ -167,14 +160,3 @@ def _handle_terminator(self, inst: IRInstruction): for op in self._merge(orders): if op not in self.stack: self._add_needed(op) - - def _handle_phi(self, inst: IRInstruction): - assert inst.opcode == "phi" - assert inst.output is not None - - out_var = inst.output - self.translates[out_var] = dict() - - for label, var in inst.phi_operands: - assert isinstance(var, IRVariable) - self.translates[out_var][label] = var From 9b5561c772a2904c1ca2826378c35e2f76950f71 Mon Sep 17 00:00:00 2001 From: Hodan Date: Wed, 3 Sep 2025 17:03:43 +0200 Subject: [PATCH 47/47] added back xfail --- tests/unit/compiler/venom/test_stack_order.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/compiler/venom/test_stack_order.py b/tests/unit/compiler/venom/test_stack_order.py index 38765b0a56..17cfd063c2 100644 --- a/tests/unit/compiler/venom/test_stack_order.py +++ b/tests/unit/compiler/venom/test_stack_order.py @@ -299,6 +299,7 @@ def test_stack_order_phi(): _check_pre_post(pre, post) +@pytest.mark.xfail def test_stack_order_more_phi(): pre = """ main: