Skip to content

[Cleanup][Frontend] Dead code, style nits, and minor smells in PyTorchSimFrontend #241

Description

@YWHyuk

Summary

Checklist of low-priority cleanup items found while reviewing PyTorchSimFrontend/. None are correctness bugs on their own (the real bugs are filed separately in #236, #237, #238, #239, #240). Filing as a single tracking issue for easy batch cleanup.

Dead code

  • extension_op.py:116-179generate_inner_product_matrix is 63 lines with no call sites anywhere in the repo. Safe to delete.
  • extension_op.py:285-288sparse_mm_dummy_stonne_outer uses yield, making it a generator function. It is referenced from the wrapper header in mlir_codegen_backend.py but is never iterated, so calling it returns a generator object, not the result of out.copy_. Either drop the yield or remove the function.
  • extension_codecache.py:249-251w_offset is computed conditionally then unconditionally overwritten with 0. Drop the dead branch (commented-out original expression suggests this is a known hack).
  • mlir_codegen_backend.py:317, 1410dma_write_counter is initialized but never incremented; only dma_read_counter is bumped (line 1407). If MVOUT is always type MVOUT1, drop dma_write_counter to avoid confusion.

Copy-paste / misleading messages

  • extension_codecache.py:137-139TileSizeError.__init__ default message is "SPAD overflow occurred.", identical to SpadOverflowError's message (copy-paste). Change it to something like "Tile size constraint violated."
  • extension_codecache.py:170, 220MLIRCodeCache.load instantiates two FileLock objects against the same lock-file path with a non-locked gap between them. The on-disk file is the actual serialization primitive so there is no race, but folding both with lock: blocks under one acquisition is clearer.
  • extension_codecache.py:19-23 and mlir_autotune.py:19-23get_write_path and hash_prefix are duplicated verbatim across the two modules (the comment notes circular-import avoidance). Move both to extension_config.py and import from there.

Style / Pythonic nits

  • mlir_common.py:20, 25import sympy appears twice. Remove the second one.
  • mlir_common.py:556MLIRMultiDimTile.get_nr_dim is annotated -> str but returns len(self._tile_size), an int. Fix the annotation.
  • mlir_scheduling.py:185 — bare except: swallows KeyboardInterrupt / SystemExit. Switch to except Exception:.
  • mlir_scheduling.py:63revert_group has no return statement, but the caller does node1 = self.revert_group(node1) and then never uses node1 again. Drop the assignment or have revert_group return its argument.
  • Mutable default arguments across the codegen stack — none are currently mutated, but they are a long-running foot-gun:
    • extension_codecache.py:155, 270arg_attributes=[], tile_size=[]
    • mlir_common.py:203, 1153extra_node=dict(), affine_yield=dict()
    • mlir_scheduling.py:310origins={}
    • mlir_codegen_backend.py:435indirect_dims=[]
      Replace each with None and initialize in the body.
  • Active code paths mixing print() with the module logger. Examples:
    • mlir_codegen_backend.py:486
    • extension_config.py:159
    • extension_op.py:176-178, 187-188
    • mlir_caller_codegen.py:199-200
      These bypass log-level filtering. Route through logger (or remove if debug-only).

Duplication (templates) — defer if too invasive

The four conv templates (mlir_conv_template.py, mlir_conv_sb_template.py, mlir_conv_sbs_template.py, mlir_conv_mt_template.py) and the gemm/bmm templates share large copy-pasted blocks. Lifting the common pieces into mlir_conv_common.py / mlir_common.py would shrink the surface, but the templates also intentionally diverge (single-batch vs multi-tile vs subtile-stationary variants), so the dedup needs careful design. Filing as a note rather than an action item.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions