fix: optimizer regression (#2868)

the optimizer passes introduced in efe1dbe were too aggressive, and
would optimize out expressions that had side effects.

Author: charles-cooper

tests/compiler/ir/test_optimize_ir.py

@@ -8,6 +8,7 @@
     (["eq", 1, 2], [0]),
     (["lt", 1, 2], [1]),
     (["eq", "x", 0], ["iszero", "x"]),
+    (["eq", ["sload", 0], 0], ["iszero", ["sload", 0]]),
     # branch pruner
     (["if", ["eq", 1, 2], "pass"], ["seq"]),
     (["if", ["eq", 1, 1], 3, 4], [3]),
@@ -22,20 +23,22 @@
     (["mstore", 0, ["eq", 1, 2]], ["mstore", 0, 0]),
     # conditions
     (["ge", "x", 0], [1]),  # x >= 0 == True
+    (["ge", ["sload", 0], 0], None),  # no-op
     (["iszero", ["gt", "x", 2 ** 256 - 1]], [1]),  # x >= MAX_UINT256 == False
     (["iszero", ["sgt", "x", 2 ** 255 - 1]], [1]),  # signed x >= MAX_INT256 == False
     (["le", "x", 0], ["iszero", "x"]),
     (["le", 0, "x"], [1]),
+    (["le", 0, ["sload", 0]], None),  # no-op
     (["lt", "x", 0], [0]),
     (["lt", 0, "x"], ["iszero", ["iszero", "x"]]),
-    (["gt", 5, "x"], ["lt", "x", 5]),
-    (["ge", 5, "x"], ["le", "x", 5]),
-    (["lt", 5, "x"], ["gt", "x", 5]),
-    (["le", 5, "x"], ["ge", "x", 5]),
-    (["sgt", 5, "x"], ["slt", "x", 5]),
-    (["sge", 5, "x"], ["sle", "x", 5]),
-    (["slt", 5, "x"], ["sgt", "x", 5]),
-    (["sle", 5, "x"], ["sge", "x", 5]),
+    (["gt", 5, "x"], None),
+    (["ge", 5, "x"], None),
+    (["lt", 5, "x"], None),
+    (["le", 5, "x"], None),
+    (["sgt", 5, "x"], None),
+    (["sge", 5, "x"], None),
+    (["slt", 5, "x"], None),
+    (["sle", 5, "x"], None),
     (["slt", "x", -(2 ** 255)], ["slt", "x", -(2 ** 255)]),  # unimplemented
     # tricky conditions
     (["sgt", 2 ** 256 - 1, 0], [0]),  # -1 > 0
@@ -55,29 +58,37 @@
     (["add", 0, "x"], ["x"]),
     (["sub", "x", 0], ["x"]),
     (["sub", "x", "x"], [0]),
-    (["sub", ["sload", 0], ["sload", 0]], ["sub", ["sload", 0], ["sload", 0]]),  # no-op
-    (["sub", ["callvalue"], ["callvalue"]], ["sub", ["callvalue"], ["callvalue"]]),  # no-op
+    (["sub", ["sload", 0], ["sload", 0]], None),
+    (["sub", ["callvalue"], ["callvalue"]], None),
     (["mul", "x", 1], ["x"]),
     (["div", "x", 1], ["x"]),
     (["sdiv", "x", 1], ["x"]),
     (["mod", "x", 1], [0]),
+    (["mod", ["sload", 0], 1], None),
     (["smod", "x", 1], [0]),
     (["mul", "x", -1], ["sub", 0, "x"]),
     (["sdiv", "x", -1], ["sub", 0, "x"]),
     (["mul", "x", 0], [0]),
+    (["mul", ["sload", 0], 0], None),
     (["div", "x", 0], [0]),
+    (["div", ["sload", 0], 0], None),
     (["sdiv", "x", 0], [0]),
+    (["sdiv", ["sload", 0], 0], None),
     (["mod", "x", 0], [0]),
+    (["mod", ["sload", 0], 0], None),
     (["smod", "x", 0], [0]),
     (["mul", "x", 32], ["shl", 5, "x"]),
     (["div", "x", 64], ["shr", 6, "x"]),
     (["mod", "x", 128], ["and", "x", 127]),
-    (["sdiv", "x", 64], ["sdiv", "x", 64]),  # no-op
-    (["smod", "x", 64], ["smod", "x", 64]),  # no-op
+    (["sdiv", "x", 64], None),
+    (["smod", "x", 64], None),
     # bitwise ops
     (["shr", 0, "x"], ["x"]),
     (["sar", 0, "x"], ["x"]),
     (["shl", 0, "x"], ["x"]),
+    (["shr", 256, "x"], None),
+    (["sar", 256, "x"], None),
+    (["shl", 256, "x"], None),
     (["and", 1, 2], [0]),
     (["or", 1, 2], [3]),
     (["xor", 1, 2], [3]),
@@ -87,12 +98,13 @@
     (["or", "x", 0], ["x"]),
     (["or", 0, "x"], ["x"]),
     (["xor", "x", 0], ["x"]),
-    (["xor", "x", 1], ["xor", "x", 1]),  # no-op
-    (["and", "x", 1], ["and", "x", 1]),  # no-op
-    (["or", "x", 1], ["or", "x", 1]),  # no-op
+    (["xor", "x", 1], None),
+    (["and", "x", 1], None),
+    (["or", "x", 1], None),
     (["xor", 0, "x"], ["x"]),
     (["iszero", ["or", "x", 1]], [0]),
     (["iszero", ["or", 2, "x"]], [0]),
+    (["iszero", ["or", 1, ["sload", 0]]], None),
     # nested optimizations
     (["eq", 0, ["sub", 1, 1]], [1]),
     (["eq", 0, ["add", 2 ** 255, 2 ** 255]], [1]),  # test compile-time wrapping
@@ -108,9 +120,13 @@
 def test_ir_optimizer(ir):
     optimized = optimizer.optimize(IRnode.from_list(ir[0]))
     optimized.repr_show_gas = True
-    hand_optimized = IRnode.from_list(ir[1])
-    hand_optimized.repr_show_gas = True
-    assert optimized == hand_optimized
+    if ir[1] is None:
+        # no-op, assert optimizer does nothing
+        expected = IRnode.from_list(ir[0])
+    else:
+        expected = IRnode.from_list(ir[1])
+    expected.repr_show_gas = True
+    assert optimized == expected
 
 
 static_assertions_list = [

tests/parser/features/test_internal_call.py

@@ -51,6 +51,24 @@ def return_hash_of_rzpadded_cow() -> bytes32:
     print("Passed single fixed-size argument self-call test")
 
 
+# test that side-effecting self calls do not get optimized out
+def test_selfcall_optimizer(get_contract):
+    code = """
+counter: uint256
+
+@internal
+def increment_counter() -> uint256:
+    self.counter += 1
+    return self.counter
+@external
+def foo() -> (uint256, uint256):
+    x: uint256 = unsafe_mul(self.increment_counter(), 0)
+    return x, self.counter
+    """
+    c = get_contract(code)
+    assert c.foo() == [0, 1]
+
+
 def test_selfcall_code_3(get_contract_with_gas_estimation, keccak):
     selfcall_code_3 = """
 @internal

vyper/codegen/ir_node.py

@@ -268,7 +268,7 @@ def gas(self):
     def is_complex_ir(self):
         # list of items not to cache. note can add other env variables
         # which do not change, e.g. calldatasize, coinbase, etc.
-        do_not_cache = {"~empty"}
+        do_not_cache = {"~empty", "calldatasize"}
         return (
             isinstance(self.value, str)
             and (self.value.lower() in VALID_IR_MACROS or self.value.upper() in get_ir_opcodes())

vyper/ir/optimizer.py

@@ -39,6 +39,12 @@ def _is_int(node: IRnode) -> bool:
     return isinstance(node.value, int)
 
 
+def _deep_contains(node_or_list, node):
+    if isinstance(node_or_list, list):
+        return any(_deep_contains(t, node) for t in node_or_list)
+    return node is node_or_list
+
+
 arith = {
     "add": (operator.add, "+", UNSIGNED),
     "sub": (operator.sub, "-", UNSIGNED),
@@ -109,7 +115,7 @@ def _wrap(x, unsigned=unsigned):
         left, right = _int(args[0]), _int(args[1])
         new_val = fn(left, right)
         new_val = _wrap(new_val)
-        return False, new_val, [], new_ann
+        return new_val, [], new_ann
 
     new_val = None
     new_args = None
@@ -278,10 +284,16 @@ def _conservative_eq(x, y):
         new_val = "iszero"
         new_args = [["iszero", args[0]]]
 
-    if new_val is None:
-        return False, binop, args, ann
+    rollback = (
+        new_val is None
+        or (args[0].is_complex_ir and not _deep_contains(new_args, args[0]))
+        or (args[1].is_complex_ir and not _deep_contains(new_args, args[1]))
+    )
+
+    if rollback:
+        return None
 
-    return True, new_val, new_args, new_ann
+    return new_val, new_args, new_ann
 
 
 def optimize(node: IRnode, parent: Optional[IRnode] = None) -> IRnode:
@@ -294,6 +306,16 @@ def optimize(node: IRnode, parent: Optional[IRnode] = None) -> IRnode:
     annotation = node.annotation
     add_gas_estimate = node.add_gas_estimate
 
+    def finalize(ir_builder):
+        return IRnode.from_list(
+            ir_builder,
+            typ=typ,
+            location=location,
+            source_pos=source_pos,
+            annotation=annotation,
+            add_gas_estimate=add_gas_estimate,
+        )
+
     optimize_more = False
 
     if value == "seq":
@@ -308,7 +330,11 @@ def optimize(node: IRnode, parent: Optional[IRnode] = None) -> IRnode:
 
     elif value in arith:
         parent_op = parent.value if parent is not None else None
-        optimize_more, value, argz, annotation = _optimize_arith(value, argz, annotation, parent_op)
+
+        res = _optimize_arith(value, argz, annotation, parent_op)
+        if res is not None:
+            optimize_more = True
+            value, argz, annotation = res
 
     ###
     # BITWISE OPS
@@ -369,18 +395,12 @@ def optimize(node: IRnode, parent: Optional[IRnode] = None) -> IRnode:
             argz = []
 
     # NOTE: this is really slow (compile-time).
-    # maybe should optimize the tree in-place
-    ret = IRnode.from_list(
-        [value, *argz],
-        typ=typ,
-        location=location,
-        source_pos=source_pos,
-        annotation=annotation,
-        add_gas_estimate=add_gas_estimate,
-    )
+    # ideal would be to optimize the tree in-place
+    ret = finalize([value, *argz])
 
     if optimize_more:
         ret = optimize(ret, parent=parent)
+
     return ret