feat: enforce IR verification throughout pipeline

[DzSexton]

Summary

• make def-before-use CFG-aware, including instruction order, dominance, and explicitly declared parameters/globals
• wire --verify-ir into CompilerConfig and verify after parsing, after every optimization pass, and before code generation
• fail fast on verifier errors while preserving warnings, and declare DSL/ONNX external values explicitly

Verification

• python -m pytest tests/test_ir_verifier.py -q — 44 passed
• python -m pytest tests -q --ignore=tests/test_simulator.py --deselect=tests/test_inst_counter.py::TestCompareFiles::test_compare_two_files — 364 passed, 4 skipped, 1 deselected
• git diff --check

Notes

• The deselected test is an existing Windows-only NamedTemporaryFile handle issue in test_compare_two_files; it is unrelated to this change.
• Extended DSL joins without PHI nodes are now correctly rejected by the verifier; implementing PHI lowering is outside this PR's scope.

[github-actions]

🤖 AI Code Review

共审查 6 个变更文件

📁 scratchv/analysis/ir_verifier.py

🔴 规则7未实现 — docstring 声明了“每个函数必须包含一个基本块”，但 _verify_function 中并无检查 func.blocks 是否为空的逻辑。

🟡 重复的标签存在性检查 — _check_label_existence 和 _check_control_flow_integrity 都验证了 BR_IF 必须有恰好两个非空目标，导致同一条件下会报两次错误。建议移除其中一个。

🟡 不可达块的支配集可能导致误报 — _compute_dominators 将不可达块的 dominators 设为 {name}，导致 _check_def_before_use 在不可达块中引用外部定义时报未定义错误（如 if def_block != block.name and def_block in dominators.get(block.name, set()) 永远假）。考虑跳过不可达块或继承可达块的支配关系。

💭 类型检查丢失 NN 操作上下文 — 合并后的 _check_type_consistency 报错信息不再区分二进制和 NN 操作，对调试不友好，建议保留原提示。

💭 SSA 检查使用集合丢失位置信息 — _check_ssa_validity 改用集合 assigned 后，重复赋值时无法知道首次定义的位置，不利于理解错误。

💭 CFG 构建依赖块顺序 — _compute_dominators 中对空块或无终结指令的块采用 fallthrough 到下一个块，假设块按顺序布局，但 IR 不一定保证顺序，建议改用显式跳转边。

💭 终结指令集合重复定义 — _check_control_flow_integrity 和 _compute_dominators 各自定义了 terminators / 分支处理，建议提取为类常量。

---

📁 scratchv/compiler.py

🔴 行为变更：IR 错误从警告变为编译失败 — 当 verify_ir=True 时，解析后及优化后的 IR 错误现在终止编译，而原代码（通过 use_logger）仅将错误作为警告追加。如果用户依赖 use_logger 的宽松行为，升级后可能意外中断流水线。
建议：考虑在 verify_ir 配置下增加 strict 子选项，或保留原警告行为，仅当 verify_ir 为 True 时才硬性失败。

🟡 PassManager 可能传递 None 数据 — _IRVerificationPass 在失败时返回 PassResult(data=None)。若 PassManager.run 未检查 success 即继续传递 data，后续 pass 将收到 None 并崩溃。
建议：确认 PassManager.run 在 success=False 时立即停止流水线，或让 _IRVerificationPass 在失败时仍返回 input_data 以便后续 pass 有机会处理（但可能掩盖错误）。

🟡 警告消息格式变更 — 原 _verify_ir 生成 "IR: {msg}" 或 "IR(warning): {msg}"，新代码改为 "IR verification {phase}: {issue}"。依赖此格式的日志解析、测试断言或外部工具需要更新。
建议：保持向后兼容，或明确记录格式变更以通知用户。

💭 _IRVerificationPass 的警告丢失级别前缀 — 原代码用 issue.level.value 区分 "IR: " 与 "IR(warning): "，新代码统一使用 "IR verification" 前缀，且警告与错误消息均通过 PassResult.warnings 传递，下游无法区分 severity。
建议：在 warnings 字符串中保留 "WARNING" 或 "ERROR" 标记，或通过 PassResult 的额外字段传递级别。

---

📁 scratchv/frontend/dsl_parser.py

🔴 Bug: Implicit parameter leaks into nested functions — If current_func is a nested function, a variable from an outer scope (e.g., a closure) will be added as a parameter of the inner function on first access. This breaks encapsulation and likely produces invalid DSL.
Suggestion: Ensure _resolve only treats undiscovered names as parameters when they belong to the current function’s own scope, not to an outer one. Consider using a symbol table that tracks scopes before falling back to “create parameter”.

🟡 Potential duplicate parameter — If current_func.params is populated before this code runs (e.g., from an explicit function signature), the variable will not be re-added because _vars already contains it. But if the order is reversed, or if _vars is cleared between calls, the same variable could be appended to params multiple times.
Suggestion: Add a guard: if v not in current_func.params before appending.

💭 Test coverage — This change alters the semantics of how function inputs are resolved. Ensure unit tests cover:

• A variable used inside a function that is not declared in its signature becomes an implicit parameter.
• Inside a nested function, outer variables are NOT added to the inner function’s parameters.
• Variables used multiple times inside the same function are not added twice.

---

📁 scratchv/frontend/onnx_parser.py

🟡 Missing global registration for scalar initializers — The if arr.ndim != 0: branch now adds val to self.builder.program.global_values, but scalar initializers (ndim == 0) are not added. If scalars are also expected to be tracked globally, they should be added in the corresponding else branch. If they are intentionally excluded (e.g., because they are stored as constants elsewhere), add a comment to clarify the reasoning and avoid future confusion.

---

📁 scratchv/main.py

🟡 配置透传缺失 — 之前 --verify-ir 只定义在参数解析器中，但未传给 CompilerConfig，导致功能失效。本次修复补全了该透传，正确。

💭 help 文本可进一步明确 — 原描述“before and after optimization”改为“throughout the compiler pipeline”更准确，但若实际只在特定阶段运行，建议与实现同步。可考虑更精确的措辞，例如“Run IR verifier at key pipeline stages (pre-opt, post-opt, final)”。

---

📁 tests/test_ir_verifier.py

🔴 test_empty_block_falls_through_for_dominance — Line 299: creates a gap block with no terminator, then verifies. If the verifier requires every block to have a terminator (block-termination rule is ERROR level), verify_ir will return passed=False and the test fails. The test currently assumes empty blocks are allowed and fall through, but this is not guaranteed. Either add a BR target to gap, or explicitly expect a block-termination warning/error.

🟡 test_duplicate_block_labels_are_rejected — Line 338: directly creates BasicBlock("duplicate") objects and extends func.blocks without an entry block. This may cause an unrelated "entry-existence" error, which doesn't affect the label check but pollutes the test. Consider adding a real entry block first to keep the test focused.

🟡 Code duplication — make_warning_only_program() is defined at line 87 but never used. test_verify_ir_allows_warning_only_program (line 728) manually constructs the same program. Reuse the helper to reduce duplication.

💭 Fragile mock for ONNX — test_onnx_tensor_initializer_is_a_program_global (line 840) stubs sys.modules["onnx"] with a SimpleNamespace that only provides load and numpy_helper. If ONNXParser.parse internally references other attributes (e.g., onnx.TensorProto, onnx.GraphProto), the test will break. Consider using a more robust fixture (e.g., unittest.mock.patch with a real ONNX model string) or adding a note about the stubbed interface.

💭 Monkeypatching IRVerifier.verify — test_verifies_after_every_optimization_pass (line 790) patches IRVerifier.verify globally. This can interfere with other tests if they run in the same process (e.g., if tests are not isolated). While monkeypatch is fixture-scoped, it's worth noting that the patch is broad and could affect concurrent test runs. Consider using a more targeted approach (e.g., subclassing IRVerifier).

---