From 56369c201ae84fcb96f4fdae1eee8b3bdfa3f89d Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Tue, 30 Apr 2024 11:32:51 +0300 Subject: [PATCH 1/7] add test cases --- tests/unit/compiler/asm/test_asm_optimizer.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unit/compiler/asm/test_asm_optimizer.py b/tests/unit/compiler/asm/test_asm_optimizer.py index 5742f7c8df..66723e8dd7 100644 --- a/tests/unit/compiler/asm/test_asm_optimizer.py +++ b/tests/unit/compiler/asm/test_asm_optimizer.py @@ -3,6 +3,7 @@ from vyper.compiler import compile_code from vyper.compiler.phases import CompilerData from vyper.compiler.settings import OptimizationLevel, Settings +from vyper.ir.compile_ir import _merge_jumpdests codes = [ """ @@ -123,3 +124,15 @@ def foo(): assert "unused1()" not in asm assert "unused2()" not in asm + + +asms = [ + ["_sym_label_1", "JUMP" "PUSH0", "_sym_label_1", "JUMPDEST", "_sym_label_0", "JUMPDEST"], + ["_sym_label_0", "JUMP" "PUSH0", "_sym_label_0", "JUMPDEST", "_sym_label_0", "JUMPDEST"], +] + + +@pytest.mark.parametrize("asm", asms) +def test_merge_jumpdests(asm): + assert _merge_jumpdests(asm) == True + assert _merge_jumpdests(asm) == False, "Should not 'merge jumpdests' again" From d246f16298a27f217b3eb896fd989a4b78bf2b79 Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Tue, 30 Apr 2024 11:47:15 +0300 Subject: [PATCH 2/7] fix symbol replacement bug in _merge_jumpdests function --- vyper/ir/compile_ir.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/vyper/ir/compile_ir.py b/vyper/ir/compile_ir.py index 191803295e..526ec8c4be 100644 --- a/vyper/ir/compile_ir.py +++ b/vyper/ir/compile_ir.py @@ -892,10 +892,14 @@ def _merge_jumpdests(assembly): # replace all instances of _sym_x with _sym_y # (except for _sym_x JUMPDEST - don't want duplicate labels) new_symbol = assembly[i + 2] - for j in range(len(assembly)): - if assembly[j] == current_symbol and i != j: - assembly[j] = new_symbol - changed = True + if new_symbol == current_symbol: + del assembly[i : i + 2] + changed = True + else: + for j in range(len(assembly)): + if assembly[j] == current_symbol and i != j: + assembly[j] = new_symbol + changed = True elif is_symbol(assembly[i + 2]) and assembly[i + 3] == "JUMP": # _sym_x JUMPDEST _sym_y JUMP # replace all instances of _sym_x with _sym_y From 9f85f2aac94073f70225303dc79d54ccc0e004e4 Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Tue, 30 Apr 2024 11:49:12 +0300 Subject: [PATCH 3/7] add comment --- vyper/ir/compile_ir.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vyper/ir/compile_ir.py b/vyper/ir/compile_ir.py index 526ec8c4be..dbeeb1e92a 100644 --- a/vyper/ir/compile_ir.py +++ b/vyper/ir/compile_ir.py @@ -893,6 +893,8 @@ def _merge_jumpdests(assembly): # (except for _sym_x JUMPDEST - don't want duplicate labels) new_symbol = assembly[i + 2] if new_symbol == current_symbol: + # no need to replace if they are the same + # just delete the extra JUMPDEST del assembly[i : i + 2] changed = True else: From b4cccc4f5ef84c5f4857050c3fab35fb370bf83d Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Tue, 30 Apr 2024 12:00:34 +0300 Subject: [PATCH 4/7] lint --- tests/unit/compiler/asm/test_asm_optimizer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/compiler/asm/test_asm_optimizer.py b/tests/unit/compiler/asm/test_asm_optimizer.py index 66723e8dd7..d0616c0986 100644 --- a/tests/unit/compiler/asm/test_asm_optimizer.py +++ b/tests/unit/compiler/asm/test_asm_optimizer.py @@ -134,5 +134,5 @@ def foo(): @pytest.mark.parametrize("asm", asms) def test_merge_jumpdests(asm): - assert _merge_jumpdests(asm) == True - assert _merge_jumpdests(asm) == False, "Should not 'merge jumpdests' again" + assert _merge_jumpdests(asm) is True + assert _merge_jumpdests(asm) is False, "Should not 'merge jumpdests' again" From 802641e1bc6c4ab9bb06983cd08429619e4700fa Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Wed, 1 May 2024 14:34:51 +0300 Subject: [PATCH 5/7] convert to just return false as no change made --- tests/unit/compiler/asm/test_asm_optimizer.py | 15 +++++---------- vyper/ir/compile_ir.py | 7 +------ 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/tests/unit/compiler/asm/test_asm_optimizer.py b/tests/unit/compiler/asm/test_asm_optimizer.py index d0616c0986..336bbeac71 100644 --- a/tests/unit/compiler/asm/test_asm_optimizer.py +++ b/tests/unit/compiler/asm/test_asm_optimizer.py @@ -126,13 +126,8 @@ def foo(): assert "unused2()" not in asm -asms = [ - ["_sym_label_1", "JUMP" "PUSH0", "_sym_label_1", "JUMPDEST", "_sym_label_0", "JUMPDEST"], - ["_sym_label_0", "JUMP" "PUSH0", "_sym_label_0", "JUMPDEST", "_sym_label_0", "JUMPDEST"], -] - - -@pytest.mark.parametrize("asm", asms) -def test_merge_jumpdests(asm): - assert _merge_jumpdests(asm) is True - assert _merge_jumpdests(asm) is False, "Should not 'merge jumpdests' again" +def test_merge_jumpdests(): + asm = ( + ["_sym_label_0", "JUMP" "PUSH0", "_sym_label_0", "JUMPDEST", "_sym_label_0", "JUMPDEST"], + ) + assert _merge_jumpdests(asm) is False, "should not return True as no changes were made" diff --git a/vyper/ir/compile_ir.py b/vyper/ir/compile_ir.py index dbeeb1e92a..472d28f4fb 100644 --- a/vyper/ir/compile_ir.py +++ b/vyper/ir/compile_ir.py @@ -892,12 +892,7 @@ def _merge_jumpdests(assembly): # replace all instances of _sym_x with _sym_y # (except for _sym_x JUMPDEST - don't want duplicate labels) new_symbol = assembly[i + 2] - if new_symbol == current_symbol: - # no need to replace if they are the same - # just delete the extra JUMPDEST - del assembly[i : i + 2] - changed = True - else: + if new_symbol != current_symbol: for j in range(len(assembly)): if assembly[j] == current_symbol and i != j: assembly[j] = new_symbol From 148afff4899adebaec1691168f9b4c95b71f9fb4 Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Wed, 1 May 2024 22:42:26 +0300 Subject: [PATCH 6/7] lint fight --- tests/unit/compiler/asm/test_asm_optimizer.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/unit/compiler/asm/test_asm_optimizer.py b/tests/unit/compiler/asm/test_asm_optimizer.py index 336bbeac71..1e05ac434c 100644 --- a/tests/unit/compiler/asm/test_asm_optimizer.py +++ b/tests/unit/compiler/asm/test_asm_optimizer.py @@ -127,7 +127,6 @@ def foo(): def test_merge_jumpdests(): - asm = ( - ["_sym_label_0", "JUMP" "PUSH0", "_sym_label_0", "JUMPDEST", "_sym_label_0", "JUMPDEST"], - ) + asm = ["_sym_label_0", "JUMP" "PUSH0", "_sym_label_0", "JUMPDEST", "_sym_label_0", "JUMPDEST"] + assert _merge_jumpdests(asm) is False, "should not return True as no changes were made" From 337f38bfc25eea7f6fec2269aeb8ed8557930625 Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Wed, 1 May 2024 22:46:19 +0300 Subject: [PATCH 7/7] add comma --- tests/unit/compiler/asm/test_asm_optimizer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/compiler/asm/test_asm_optimizer.py b/tests/unit/compiler/asm/test_asm_optimizer.py index 1e05ac434c..ee6b1653b0 100644 --- a/tests/unit/compiler/asm/test_asm_optimizer.py +++ b/tests/unit/compiler/asm/test_asm_optimizer.py @@ -127,6 +127,6 @@ def foo(): def test_merge_jumpdests(): - asm = ["_sym_label_0", "JUMP" "PUSH0", "_sym_label_0", "JUMPDEST", "_sym_label_0", "JUMPDEST"] + asm = ["_sym_label_0", "JUMP", "PUSH0", "_sym_label_0", "JUMPDEST", "_sym_label_0", "JUMPDEST"] assert _merge_jumpdests(asm) is False, "should not return True as no changes were made"