From 90dc9e1ea065ab00776f57df24b467255968a000 Mon Sep 17 00:00:00 2001 From: dosisod <39638017+dosisod@users.noreply.github.com> Date: Sun, 14 May 2023 22:16:04 -0700 Subject: [PATCH 1/2] [mypyc] Remove unused labels and gotos: This PR removes labels which are never jumped to, as well as removing gotos that jump directly to the next label (and removes the now redundant label). I had a small issue with the `GetAttr` op code as it would insert C code after the end of the branch which could not be detected at the IR level, as seen in https://github.com/mypyc/mypyc/issues/600#issuecomment-1319530780 . Here are some basic stats on the file sizes before and after this PR: Building with `MYPY_OPT_LEVEL=0`, `MYPY_DEBUG_LEVEL=1`: | Branch | `.so` size | `.c` size | `.c` lines | |----------|-----------------|-----------------|-----------------| | `master` | 43.9 MB | 79.7 MB | 2290675 | | This PR | 43.6 MB (-0.6%) | 78.6 MB (-1.4%) | 2179967 (-4.8%) | Building with `MYPY_OPT_LEVEL=3`, `MYPY_DEBUG_LEVEL=0`: | Branch | `.so` size | `.c` size | `.c` lines | |----------|------------|-----------|------------| | `master` | 26.5 MB | 79.7 MB | 2291316 | | This PR | 26.5 MB | 78.7 MB | 2179701 | (I don't know why the number of lines changes between optimization levels. It's only a couple hundred lines, perhaps Mypyc is emitting different code depending on optimization level?) Unoptimized builds seem to benefit the most, while optimized builds look prety much the same. I didn't check to see if they are *exactly* the same binary, but they are probably close if not the same. The biggest win is the drop in the number of lines C code being generated. Closes https://github.com/mypyc/mypyc/issues/600 --- mypyc/codegen/emit.py | 3 +++ mypyc/codegen/emitfunc.py | 31 +++++++++++++++++++++++++++---- mypyc/ir/ops.py | 1 + mypyc/test/test_emitfunc.py | 8 +------- 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/mypyc/codegen/emit.py b/mypyc/codegen/emit.py index 56ce9637307b..a2e3d9849dca 100644 --- a/mypyc/codegen/emit.py +++ b/mypyc/codegen/emit.py @@ -229,6 +229,9 @@ def emit_label(self, label: BasicBlock | str) -> None: if isinstance(label, str): text = label else: + if label.label == 0 or not label.referenced: + return + text = self.label(label) # Extra semicolon prevents an error when the next line declares a tempvar self.fragments.append(f"{text}: ;\n") diff --git a/mypyc/codegen/emitfunc.py b/mypyc/codegen/emitfunc.py index f2406ff1a257..e37c2825c55f 100644 --- a/mypyc/codegen/emitfunc.py +++ b/mypyc/codegen/emitfunc.py @@ -23,6 +23,7 @@ CallC, Cast, ComparisonOp, + ControlOp, DecRef, Extend, Float, @@ -123,6 +124,25 @@ def generate_native_function( for i, block in enumerate(blocks): block.label = i + for block in fn.blocks: + terminator = block.terminator + assert isinstance(terminator, ControlOp) + + for bb in terminator.targets(): + is_next_block = bb.label == block.label + 1 + + # Always emit labels for GetAttr op codes since the emit code that + # generates them will add instructions between the branch and the + # next label, causing the label to be wrongly removed. A better + # solution would be to change the IR so that it adds a basic block + # inbetween the calls. + is_problematic_op = isinstance(terminator, Branch) and any( + isinstance(s, GetAttr) for s in terminator.sources() + ) + + if not is_next_block or is_problematic_op: + fn.blocks[bb.label].referenced = True + common = frequently_executed_blocks(fn.blocks[0]) for i in range(len(blocks)): @@ -216,7 +236,8 @@ def visit_branch(self, op: Branch) -> None: if false is self.next_block: if op.traceback_entry is None: - self.emit_line(f"if ({cond}) goto {self.label(true)};") + if true is not self.next_block: + self.emit_line(f"if ({cond}) goto {self.label(true)};") else: self.emit_line(f"if ({cond}) {{") self.emit_traceback(op) @@ -224,9 +245,11 @@ def visit_branch(self, op: Branch) -> None: else: self.emit_line(f"if ({cond}) {{") self.emit_traceback(op) - self.emit_lines( - "goto %s;" % self.label(true), "} else", " goto %s;" % self.label(false) - ) + + if true is not self.next_block: + self.emit_line("goto %s;" % self.label(true)) + + self.emit_lines("} else", " goto %s;" % self.label(false)) def visit_return(self, op: Return) -> None: value_str = self.reg(op.value) diff --git a/mypyc/ir/ops.py b/mypyc/ir/ops.py index 351f7c01efe2..6007f8a4ce04 100644 --- a/mypyc/ir/ops.py +++ b/mypyc/ir/ops.py @@ -81,6 +81,7 @@ def __init__(self, label: int = -1) -> None: self.label = label self.ops: list[Op] = [] self.error_handler: BasicBlock | None = None + self.referenced = False @property def terminated(self) -> bool: diff --git a/mypyc/test/test_emitfunc.py b/mypyc/test/test_emitfunc.py index d7dcf3be532b..ab1586bb22a8 100644 --- a/mypyc/test/test_emitfunc.py +++ b/mypyc/test/test_emitfunc.py @@ -894,12 +894,7 @@ def test_simple(self) -> None: generate_native_function(fn, emitter, "prog.py", "prog") result = emitter.fragments assert_string_arrays_equal( - [ - "CPyTagged CPyDef_myfunc(CPyTagged cpy_r_arg) {\n", - "CPyL0: ;\n", - " return cpy_r_arg;\n", - "}\n", - ], + ["CPyTagged CPyDef_myfunc(CPyTagged cpy_r_arg) {\n", " return cpy_r_arg;\n", "}\n"], result, msg="Generated code invalid", ) @@ -922,7 +917,6 @@ def test_register(self) -> None: [ "PyObject *CPyDef_myfunc(CPyTagged cpy_r_arg) {\n", " CPyTagged cpy_r_r0;\n", - "CPyL0: ;\n", " cpy_r_r0 = 10;\n", " CPy_Unreachable();\n", "}\n", From 66538810d492ee471f42b36f130a3c5df60be809 Mon Sep 17 00:00:00 2001 From: dosisod <39638017+dosisod@users.noreply.github.com> Date: Wed, 17 May 2023 23:12:52 -0700 Subject: [PATCH 2/2] Add requested changes --- mypyc/codegen/emitfunc.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/mypyc/codegen/emitfunc.py b/mypyc/codegen/emitfunc.py index e37c2825c55f..0dd8efac640f 100644 --- a/mypyc/codegen/emitfunc.py +++ b/mypyc/codegen/emitfunc.py @@ -124,14 +124,17 @@ def generate_native_function( for i, block in enumerate(blocks): block.label = i + # Find blocks that are never jumped to or are only jumped to from the + # block directly above it. This allows for more labels and gotos to be + # eliminated during code generation. for block in fn.blocks: terminator = block.terminator assert isinstance(terminator, ControlOp) - for bb in terminator.targets(): - is_next_block = bb.label == block.label + 1 + for target in terminator.targets(): + is_next_block = target.label == block.label + 1 - # Always emit labels for GetAttr op codes since the emit code that + # Always emit labels for GetAttr error checks since the emit code that # generates them will add instructions between the branch and the # next label, causing the label to be wrongly removed. A better # solution would be to change the IR so that it adds a basic block @@ -141,7 +144,7 @@ def generate_native_function( ) if not is_next_block or is_problematic_op: - fn.blocks[bb.label].referenced = True + fn.blocks[target.label].referenced = True common = frequently_executed_blocks(fn.blocks[0])