diff --git a/vyper/codegen/dfg.py b/vyper/codegen/dfg.py index 6e3837b3fc..c604982036 100644 --- a/vyper/codegen/dfg.py +++ b/vyper/codegen/dfg.py @@ -259,6 +259,7 @@ def _generate_evm_for_basicblock_r( label_counter = 0 +# REVIEW: would this be better as a class? def _generate_evm_for_instruction_r( ctx: IRFunction, assembly: list, inst: IRInstruction, stack_map: StackMap ) -> list[str]: @@ -266,8 +267,10 @@ def _generate_evm_for_instruction_r( for op in inst.get_output_operands(): for target in ctx.dfg_inputs.get(op.value, []): + # REVIEW: what does this line do? if target.parent != inst.parent: continue + # REVIEW: what does this line do? if target.fen != inst.fen: continue assembly = _generate_evm_for_instruction_r(ctx, assembly, target, stack_map) @@ -298,6 +301,8 @@ def _generate_evm_for_instruction_r( if opcode == "select": # REVIEW: maybe call this 'phi' ret = inst.get_output_operands()[0] inputs = inst.get_input_operands() + # REVIEW: the special handling in get_depth_in for lists + # seems cursed, refactor depth = stack_map.get_depth_in(inputs) assert depth is not StackMap.NOT_IN_STACK, "Operand not in stack" to_be_replaced = stack_map.peek(depth) diff --git a/vyper/compiler/utils.py b/vyper/compiler/utils.py index fc86fdcf38..4056575260 100644 --- a/vyper/compiler/utils.py +++ b/vyper/compiler/utils.py @@ -49,9 +49,12 @@ def _expand_row(row): return result +# REVIEW: move this to vyper/ir/ or vyper/venom/ +# rename to StackModel class StackMap: NOT_IN_STACK = object() - stack_map: list[IRValueBase] + stack_map: list[IRValueBase] # REVIEW: rename to stack + # REVIEW: dead variable dependant_liveness: OrderedSet[IRValueBase] def __init__(self): @@ -93,12 +96,15 @@ def get_depth_in(self, op: IRValueBase | list[IRValueBase]) -> int: for i, stack_op in enumerate(self.stack_map[::-1]): if isinstance(stack_op, IRValueBase): + # REVIEW: handling literals this way seems a bit cursed, + # why not use IRLiteral, so it is always IRValueBase? if isinstance(op, str) and stack_op.value == op: return -i if isinstance(op, int) and stack_op.value == op: return -i if isinstance(op, IRValueBase) and stack_op.value == op.value: return -i + # REVIEW: this branch seems cursed elif isinstance(op, list) and stack_op in op: return -i