From 0113721cbbf041f910b31db81a42ec02c8db7f09 Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Sun, 7 Apr 2024 00:51:32 +0300 Subject: [PATCH] refactor[venom]: refactor venom operand classes (#3915) the original `IRValue`, `IRVariable`, `IRLabel` and `IROperand` hierarchy got convoluted after several refactors. this commit refactors the hierarchy so that the relationship between the classes is clearer. --- vyper/venom/basicblock.py | 63 ++++++++++------------------------ vyper/venom/passes/make_ssa.py | 4 +-- 2 files changed, 21 insertions(+), 46 deletions(-) diff --git a/vyper/venom/basicblock.py b/vyper/venom/basicblock.py index 6c509d8f95..0993fb0515 100644 --- a/vyper/venom/basicblock.py +++ b/vyper/venom/basicblock.py @@ -92,8 +92,8 @@ def __repr__(self) -> str: class IROperand: """ - IROperand represents an operand in IR. An operand is anything that can - be an argument to an IRInstruction + IROperand represents an IR operand. An operand is anything that can be + operated by instructions. It can be a literal, a variable, or a label. """ value: Any @@ -102,18 +102,19 @@ class IROperand: def name(self) -> str: return self.value + def __hash__(self) -> int: + return hash(self.value) -class IRValue(IROperand): - """ - IRValue represents a value in IR. A value is anything that can be - operated by non-control flow instructions. That is, IRValues can be - IRVariables or IRLiterals. - """ + def __eq__(self, other) -> bool: + if not isinstance(other, type(self)): + return False + return self.value == other.value - pass + def __repr__(self) -> str: + return str(self.value) -class IRLiteral(IRValue): +class IRLiteral(IROperand): """ IRLiteral represents a literal in IR """ @@ -124,25 +125,13 @@ def __init__(self, value: int) -> None: assert isinstance(value, int), "value must be an int" self.value = value - def __hash__(self) -> int: - return self.value.__hash__() - - def __eq__(self, other) -> bool: - if not isinstance(other, type(self)): - return False - return self.value == other.value - - def __repr__(self) -> str: - return str(self.value) - -class IRVariable(IRValue): +class IRVariable(IROperand): """ IRVariable represents a variable in IR. A variable is a string that starts with a %. """ value: str - offset: int = 0 def __init__(self, value: str, version: Optional[str | int] = None) -> None: assert isinstance(value, str) @@ -153,7 +142,6 @@ def __init__(self, value: str, version: Optional[str | int] = None) -> None: if value[0] != "%": value = f"%{value}" self.value = value - self.offset = 0 @property def name(self) -> str: @@ -165,17 +153,6 @@ def version(self) -> int: return 0 return int(self.value.split(":")[1]) - def __hash__(self) -> int: - return self.value.__hash__() - - def __eq__(self, other) -> bool: - if not isinstance(other, type(self)): - return False - return self.value == other.value - - def __repr__(self) -> str: - return self.value - class IRLabel(IROperand): """ @@ -192,16 +169,14 @@ def __init__(self, value: str, is_symbol: bool = False) -> None: self.value = value self.is_symbol = is_symbol - def __hash__(self) -> int: - return hash(self.value) + def __eq__(self, other): + # no need for is_symbol to participate in equality + return super().__eq__(other) - def __eq__(self, other) -> bool: - if not isinstance(other, type(self)): - return False - return self.value == other.value - - def __repr__(self) -> str: - return self.value + def __hash__(self): + # __hash__ is required when __eq__ is overridden -- + # https://docs.python.org/3/reference/datamodel.html#object.__hash__ + return super().__hash__() class IRInstruction: diff --git a/vyper/venom/passes/make_ssa.py b/vyper/venom/passes/make_ssa.py index 06c61c9ea7..91611482a2 100644 --- a/vyper/venom/passes/make_ssa.py +++ b/vyper/venom/passes/make_ssa.py @@ -95,7 +95,7 @@ def _rename_vars(self, basic_block: IRBasicBlock): # Pre-action for inst in basic_block.instructions: - new_ops = [] + new_ops: list[IROperand] = [] if inst.opcode != "phi": for op in inst.operands: if not isinstance(op, IRVariable): @@ -143,7 +143,7 @@ def _remove_degenerate_phis(self, entry: IRBasicBlock): if inst.opcode != "phi": continue - new_ops = [] + new_ops: list[IROperand] = [] for label, op in inst.phi_operands: if op == inst.output: continue