fix[venom]: fix list of volatile instructions (#4065)

`create` and `create2` are volatile instructions that were missing. the
other ones were already handled by the case for `output is None` in
`removed_unused_variables`.

misc:
- rename `volatile` to `is_volatile` for uniformity
- use `is_bb_terminator` instead of `in BB_TERMINATORS`

---------

Co-authored-by: Harry Kalogirou <[email protected]>

vyper/venom/analysis/cfg.py

@@ -1,6 +1,6 @@
 from vyper.utils import OrderedSet
 from vyper.venom.analysis.analysis import IRAnalysis
-from vyper.venom.basicblock import BB_TERMINATORS, CFG_ALTERING_INSTRUCTIONS
+from vyper.venom.basicblock import CFG_ALTERING_INSTRUCTIONS
 
 
 class CFGAnalysis(IRAnalysis):
@@ -18,9 +18,7 @@ def analyze(self) -> None:
         for bb in fn.get_basic_blocks():
             assert len(bb.instructions) > 0, "Basic block should not be empty"
             last_inst = bb.instructions[-1]
-            assert (
-                last_inst.opcode in BB_TERMINATORS
-            ), f"Last instruction should be a terminator {bb}"
+            assert last_inst.is_bb_terminator, f"Last instruction should be a terminator {bb}"
 
             for inst in bb.instructions:
                 if inst.opcode in CFG_ALTERING_INSTRUCTIONS:

vyper/venom/basicblock.py

@@ -12,6 +12,8 @@
         "call",
         "staticcall",
         "delegatecall",
+        "create",
+        "create2",
         "invoke",
         "sload",
         "sstore",
@@ -34,14 +36,22 @@
         "ret",
         "jmp",
         "jnz",
+        "djmp",
+        "log",
+        "selfdestruct",
+        "invalid",
+        "revert",
+        "assert",
+        "assert_unreachable",
+        "stop",
+        "exit",
     ]
 )
 
 NO_OUTPUT_INSTRUCTIONS = frozenset(
     [
         "mstore",
         "sstore",
-        "dstore",
         "istore",
         "tstore",
         "dloadbytes",
@@ -67,6 +77,10 @@
     ]
 )
 
+assert VOLATILE_INSTRUCTIONS.issuperset(NO_OUTPUT_INSTRUCTIONS), (
+    NO_OUTPUT_INSTRUCTIONS - VOLATILE_INSTRUCTIONS
+)
+
 CFG_ALTERING_INSTRUCTIONS = frozenset(["jmp", "djmp", "jnz"])
 
 if TYPE_CHECKING:
@@ -221,9 +235,13 @@ def __init__(
         self.error_msg = None
 
     @property
-    def volatile(self) -> bool:
+    def is_volatile(self) -> bool:
         return self.opcode in VOLATILE_INSTRUCTIONS
 
+    @property
+    def is_bb_terminator(self) -> bool:
+        return self.opcode in BB_TERMINATORS
+
     def get_label_operands(self) -> Iterator[IRLabel]:
         """
         Get all labels in instruction.
@@ -499,7 +517,7 @@ def is_terminated(self) -> bool:
         # if we can/need to append instructions to the basic block.
         if len(self.instructions) == 0:
             return False
-        return self.instructions[-1].opcode in BB_TERMINATORS
+        return self.instructions[-1].is_bb_terminator
 
     @property
     def is_terminal(self) -> bool:

vyper/venom/passes/dft.py

@@ -1,6 +1,6 @@
 from vyper.utils import OrderedSet
 from vyper.venom.analysis.dfg import DFGAnalysis
-from vyper.venom.basicblock import BB_TERMINATORS, IRBasicBlock, IRInstruction, IRVariable
+from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRVariable
 from vyper.venom.function import IRFunction
 from vyper.venom.passes.base_pass import IRPass
 
@@ -30,7 +30,7 @@ def _process_instruction_r(self, bb: IRBasicBlock, inst: IRInstruction, offset:
         self.visited_instructions.add(inst)
         self.inst_order_num += 1
 
-        if inst.opcode in BB_TERMINATORS:
+        if inst.is_bb_terminator:
             offset = len(bb.instructions)
 
         if inst.opcode == "phi":
@@ -55,7 +55,7 @@ def _process_basic_block(self, bb: IRBasicBlock) -> None:
 
         for inst in bb.instructions:
             inst.fence_id = self.fence_id
-            if inst.volatile:
+            if inst.is_volatile:
                 self.fence_id += 1
 
         # We go throught the instructions and calculate the order in which they should be executed

vyper/venom/passes/remove_unused_variables.py

@@ -31,7 +31,7 @@ def run_pass(self):
     def _process_instruction(self, inst):
         if inst.output is None:
             return
-        if inst.volatile:
+        if inst.is_volatile or inst.is_bb_terminator:
             return
         uses = self.dfg.get_uses(inst.output)
         if len(uses) > 0:

vyper/venom/venom_to_assembly.py

@@ -305,7 +305,7 @@ def _clean_unused_params(self, asm: list, bb: IRBasicBlock, stack: StackModel) -
         for i, inst in enumerate(bb.instructions):
             if inst.opcode != "param":
                 break
-            if inst.volatile and i + 1 < len(bb.instructions):
+            if inst.is_volatile and i + 1 < len(bb.instructions):
                 liveness = bb.instructions[i + 1].liveness
                 if inst.output is not None and inst.output not in liveness:
                     depth = stack.get_depth(inst.output)