Feature/strip unused procs - #4
Merged
Merged
Conversation
Add hasc/reachability.py implementing a conservative reachability-based
dead-procedure elimination pass. Wire it into the CLI with two new flags.
New CLI flags
--strip-unused-procs Remove unreachable internal procs before codegen
--strip-unused-report Print kept/removed report (implies above flag)
Reachability pass (hasc/reachability.py)
- Builds an internal call graph from Proc bodies (Call, CallStmt, MacroCall)
- Root set: PublicDecl symbols pointing to internal procs
- DFS from roots to mark reachable procedures
- Two conservative keep-all policies:
* Top-level asm block present (raw jsr/jmp may reference any label)
* No roots found (safe default; add public to opt-in to stripping)
CLI wiring (hasc/cli.py)
- Pass runs after validation, before codegen (module unchanged by default)
- --strip-unused-report prints roots, kept, and removed lists to stderr
- Feature off (no flags): zero change to generated assembly
Regression results
- 5 expected negative tests still fail; all positive examples pass
New examples (three DCE regression scenarios)
examples/strip_unused_procs_demo.has - public root, dead_a + dead_b removed
examples/strip_unused_procs_asm_safe.has - asm block keeps all (safeguard)
examples/strip_unused_procs_no_roots.has - no public export keeps all (safeguard)
- CHANGELOG.md: add entry under [Unreleased] → Added describing the pass, its three conservative safeguards, new CLI flags, and example files. - README.md: add feature bullet in Advanced Features; add usage section with CLI examples and a code snippet showing public-root pattern. - COMPILER_DEVELOPERS_GUIDE.md: insert [Reachability] stage into the Data Flow diagram; add feature to the Current Feature Set quick map.
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-in dead-code elimination pass that strips unreachable internal proc definitions via call-graph reachability, exposed through new CLI flags and documented with examples.
Key issues found (ordered by severity):
- Correctness risk:
hasc/reachability.pycurrently only treats top-levelasmblocks as “keep-all”, butasm { ... }can also appear insideprocbodies (per parser behavior). Internal procs reachable only via inline rawjsr/jmp/bsrin those asm blocks could be incorrectly stripped. - Docs mismatch: README + CHANGELOG describe roots as coming from
publicdeclarations only, but the implementation also treats top-levelcallstatements and top-level macro calls as roots. - Minor: quadratic filtering behavior in
reachability.py(list membership) and a small Makefile robustness issue with quoting$(PYTHON).
Changes:
- Adds
hasc/reachability.pyto compute reachability and strip unused internal procedures (opt-in). - Adds
--strip-unused-procsand--strip-unused-reportflags tohasc.cli. - Updates documentation/changelog and adds example
.hasfiles demonstrating stripping behavior; enables stripping/reporting in the caveride example Makefile.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the new stripping flags and includes usage examples. |
| hasc/reachability.py | Implements call-graph reachability and AST pruning for unused internal procs. |
| hasc/cli.py | Wires reachability pass into the pipeline + adds CLI/report flags. |
| examples/strip_unused_procs_demo.has | Demonstrates stripping with explicit public root. |
| examples/strip_unused_procs_asm_safe.has | Demonstrates the asm safeguard behavior (keep-all). |
| examples/strip_unused_procs_no_roots.has | Demonstrates conservative “no roots → keep all” scenario. |
| examples/games/caveride/Makefile | Enables stripping/reporting flags in the build, adds optional link-symbol stripping. |
| docs/STRUCT_POINTER_IMPLEMENTATION.md | Formatting/heading cleanup. |
| docs/PYTHON_INTEGRATION.md | Formatting/heading cleanup + wording tweaks. |
| docs/PYTHON_GENERATION_TUTORIAL.md | Formatting/wording tweaks. |
| docs/DEVELOPERS_GUIDE.md | Removes “Phase X” wording from feature headings. |
| docs/COMPILER_DEVELOPERS_GUIDE.md | Updates pipeline diagram + notes reachability pass and flag. |
| docs/CHANGELOG.md | Adds changelog entry describing the new stripping feature. |
Comment on lines
+64
to
+76
| def _collect_direct_calls_from_stmt(stmt, out_calls: Set[str]) -> None: | ||
| if isinstance(stmt, ast.CallStmt): | ||
| out_calls.add(stmt.name) | ||
| for arg in stmt.args: | ||
| _collect_direct_calls_from_expr(arg, out_calls) | ||
| return | ||
|
|
||
| if isinstance(stmt, ast.MacroCall): | ||
| # MacroCall can be either a macro expansion or an implicit function call. | ||
| out_calls.add(stmt.name) | ||
| for arg in stmt.args: | ||
| _collect_direct_calls_from_expr(arg, out_calls) | ||
| return |
Comment on lines
+144
to
+148
| if isinstance(code_item, ast.Proc): | ||
| proc_calls: Set[str] = set() | ||
| for stmt in code_item.body: | ||
| _collect_direct_calls_from_stmt(stmt, proc_calls) | ||
| graph[code_item.name] = set(c for c in proc_calls if c in internal_procs) |
Comment on lines
+218
to
+228
| new_items = [] | ||
| for item in module.items: | ||
| if not isinstance(item, ast.CodeSection): | ||
| new_items.append(item) | ||
| continue | ||
|
|
||
| filtered = [] | ||
| for code_item in item.items: | ||
| if isinstance(code_item, ast.Proc) and code_item.name in removed: | ||
| continue | ||
| filtered.append(code_item) |
Comment on lines
+239
to
+241
| The pass uses call-graph reachability from `public` declarations. It is | ||
| **conservative by default**: if no roots are found, or if a top-level raw | ||
| `asm` block is present, all procedures are kept unchanged. |
|
|
||
| - **Dead-procedure elimination pass** (`--strip-unused-procs` / `--strip-unused-report`): | ||
| - New module `hasc/reachability.py` performs conservative call-graph analysis after validation and before code generation. | ||
| - Roots are discovered from `public` declarations that point to internal `proc` definitions. |
| @@ -26,6 +26,7 @@ VASM := vasmm68k_mot | |||
| VLINK := vlink | |||
| WHICH := which | |||
| HASC := $(PYTHON) -m hasc.cli | |||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.