topic 17 LinearScanAllocator Benchmark

[feiji111]

依据设计文档，设计了三个Benchmark：

1. 简单算术：3-5 个 vreg 的基本算术运算，验证无溢出时的分配正确性
2. 高密度变量：30 个 vreg 在 5 个物理寄存器上运行，验证溢出逻辑
3. CNN Conv2D 集成：将分配器接入 CNN 编译管线，验证生成的汇编能被 Spike 仿真正确执行

可能的问题：
Benchmark是基于v1.3的寄存器分配算法设计(PR#37)，但是目前PR还未合并到主线

[github-actions]

🤖 AI Code Review

共审查 7 个变更文件

📁 benchmarks/test_regalloc/__init__.py

💭 Nit: 文档中引用 `models/graph/cnn.onnx`，请确认该路径在仓库中实际存在，否则会误导使用者。

💭 Nit: 包目前没有导出任何公开符号，考虑添加 `__all__ = []` 以明确意图，或说明该包仅用作测试组织。

其余无问题 —— 标准的包初始化文件，docstring 清晰，future import 正确。

---

📁 benchmarks/test_regalloc/bench_cnn.py

🔴 Bug: 性能隐患 — _compile_onnx 在函数内部反复 import 模块，每次调用都会重新加载。建议将 from scratchv.frontend.onnx_parser import ONNXParser 等移到文件顶部。

🔴 Bug: 异常捕获过宽 — _run_emulator 和 _llvm_compare 中使用 except Exception，可能掩盖真实错误（如 ImportError 导致 _KNOWN_OPS 未定义）。建议缩小异常类型或记录完整 traceback。

🟡 可维护性: import 散落各处 — from .bench_utils import _KNOWN_OPS 等出现在文件中间，违背 PEP8 导入顺序规范。建议将所有 import 集中在文件顶部。

🟡 可维护性: 残留注释 — # lib = _load_llvm() 在第 131 行被注释掉，应删除以免混淆。

🟡 性能: 调用私有属性 — alloc._spill_slots 和 alloc.peak_active 直接访问私有成员。建议提供公共属性或方法。

🟡 逻辑歧义: IR 计数时机 — ir_count 在 optimizer 之前计算，但 optimizer 可能删除指令，造成统计不一致。建议在 optimizer 之后重新计算。

🟡 可维护性: 冗余赋值 — stats["valid"] = stats["asm_valid"] 是多余的，可以直接复用 stats["asm_valid"]。

💭 风格: sys.exit(main() or 0) — 可简化为 sys.exit(main())，因为 main() 返回 0 或 1。

💭 命名: 变量名 machine 含义模糊 — _compile_onnx 返回的 machine 实际上是 List[MachineInstr]，建议更名为 machine_instrs。

💭 文档: 缺少 _validate_asm 的测试 — 该函数逻辑复杂，建议在 benchmark 之外增加单元测试覆盖。

---

📁 benchmarks/test_regalloc/bench_dense.py

🔴 Bug: 依赖未公开的私有属性 — 第 76、77、78 行访问 alloc._spill_slots、alloc.alloc_map、alloc.peak_active。这些属性可能不存在或接口变化导致运行时错误。建议：在 LinearScanAllocator 中添加公共 getter（如 spill_slots()、alloc_map()、peak_active()）或直接作为公开属性。

🔴 Bug: peak_active 可能未定义 — 第 78 行的 alloc.peak_active 在提供的代码中未定义，若 LinearScanAllocator 未实现该属性，会引发 AttributeError。建议：确认该属性存在，否则改为 getattr(alloc, 'peak_active', 0) 或添加相应属性。

🟡 建议: 全局随机状态污染 — 第 17 行 random.seed(seed) 修改全局 random 模块状态，可能影响其他依赖随机数的模块。建议：使用 random.Random(seed) 创建独立实例。

🟡 建议: 重复测量中 spill 统计仅取末次 — 第 70–73 行循环中收集 spill_counts，但最终只返回 spill_counts[-1]（第 79 行）。虽然输入固定，但若未来生成随机 block，每次 spill 可能不同，应返回所有统计（如均值、最大/最小）以反映稳定性。

🟡 建议: 冗余键 reg_spill_count — 第 80 行 "reg_spill_count": spill_counts[-1] 与第 79 行 "spills" 完全相同，建议移除。

💭 nit: 未解释的 # flake8: noqa — 第 1 行忽略所有 flake8 检查，可能隐藏了真正的 lint 问题。建议：移除该注释，或仅忽略特定规则（如 # flake8: noqa: E501）并注明原因。

💭 nit: valid 键未使用 — 第 93 行设置 stats["valid"]，但 main 中通过 spills > 0 判断，从未读取该键。建议移除或用于输出。

💭 nit: 私有函数命名 _gen_block — 虽在模块内，但 benchmark 文件通常无强封装，可改为公开函数便于复用。

---

📁 benchmarks/test_regalloc/bench_regalloc_linear.py

🔴 KeyError risk — Lines 91, 100, 109: Direct access r1['reg_spill_count'], r1['mean_s'], r3['reg_spill_count'] etc. If any benchmark dict misses a key, script crashes. Use .get() with a sensible default (e.g., r1.get('reg_spill_count', '?')).

🔴 Missing file existence check — Line 108: cnn_default = "models/graph/cnn.onnx" is hardcoded and never checked. If the file does not exist, bench_cnn.run_bench will likely raise a file-not-found error. Add os.path.exists(cnn_default) check and print a clear error message before running.

🟡 Inconsistent dict access — Lines 91-112: Mix of r1['key'] and r1.get('valid'). Unify to .get() for all fields to avoid silent failures and improve readability.

🟡 Timestamp inconsistency — _make_html and _make_markdown each call datetime.datetime.now().isoformat() independently. Compute the timestamp once in main() and pass it to both generators for report consistency.

💭 Typo — Line 107: Comment says "Comparation" instead of "Comparison". Minor but could be confusing.

---

📁 benchmarks/test_regalloc/bench_simple.py

🔴 Bug: Inconsistent uses in _gen_block — Lines 32-44: In the if i < num_vregs branch, uses includes dst (since src1/src2 are chosen from a pool that contains dst), but the else branch explicitly subtracts dst. This inconsistency may cause the allocator to treat an instruction as using a register it defines, which could lead to incorrect live-interval computation or unexpected spills.
Suggestion: Unify the logic — either always exclude dst from uses, or document why the first branch intentionally includes it.

🟡 Fragile private access — Line 49: len(alloc._spill_slots) accesses a private attribute. If the allocator’s internals change, this benchmark breaks.
Suggestion: Add a public property spill_count to LinearScanAllocator and use that.

🟡 Redundant duplicate stats — Lines 58-59: Both spills and reg_spill_count are set to spill_counts[-1].
Suggestion: Keep only one field; the duplicate adds confusion.

🟡 Inconsistent spill source — bench_allocate runs a final allocation after the loop (line 52) but uses spill_counts[-1] from the loop for the returned spills. Since the block is deterministic, they match, but this is fragile if randomness is introduced later.
Suggestion: Use the final run’s spill count for consistency, or remove the extra run.

💭 Nit: # flake8: noqa — Line 1 disables all linting for the file, masking potential style or unused-import issues.
Suggestion: Remove it and fix any actual lint warnings.

💭 Nit: _gen_block self-use — For i=0, the instruction v0 = v0 + v0 is generated. This is a valid test case, but it’s not obvious from the code; consider a comment.

💭 Nit: Benchmark stats — The function returns _alloc and _report dicts, which expose internals and may be unnecessary for a benchmark. If they’re only for debugging, consider removing them from the public return.

---

📁 benchmarks/test_regalloc/bench_utils.py

🔴 重复定义 _CALLEE_SAVED — 第12行和第130行定义了两次相同的集合，第一个定义被覆盖且无实际用途，容易混淆。建议删除第一处定义。

🟡 冗余导入 — llvmlite_ir_to_riscv 函数中第193行（if not _llvmlite_ready 块内）已导入 from llvmlite import binding as llvm，第209行再次导入，完全多余。应移除第二次导入。

🟡 硬编码系统库路径 — libLLVM-20.so 版本号固定，运行时若系统 LLVM 版本不同会静默失败。建议使用 llvmlite 的自动绑定或尝试 ctypes.util.find_library("LLVM") 动态查找。

🟡 指令集不一致 — _CAT_MUL 包含 mulh、mulhu、mulhsu，但 _KNOWN_OPS 未收录这些指令，导致 _op_categories 分类时这些指令被归为 Other。建议同步更新 _KNOWN_OPS 或说明意图。

💭 全局禁用 flake8 检查 — 文件头 # flake8: noqa 抑制了所有检查，隐藏了潜在问题。建议改为仅对必要行添加 # noqa 注释。

💭 缺少异常处理 — target.create_target_machine() 可能因无效特征字符串或选项失败，未捕获异常，调用者无法处理。建议添加 try-except 或文档说明可能抛出的异常。

---

📁 benchmarks/test_regalloc/regalloc.md

🔴 不一致：reg_spill_count 在通用键表和说明中冲突

§4.1 通用键表说 reg_spill_count 是 len(alloc._spill_slots)（别名 spills），但 §4.3 又定义 reg_spill_count 针对非 regalloc 路径（LLVM 侧）为 0，且 LLVM 溢出独立为 llvm_spill_slots。这会导致混淆：reg_spill_count 在 JSON 中既有 "reg_spill_count": 0（§6.2 CNN 示例），又有 "llvm_spill_slots": 87。建议：

• 统一在所有 benchmark 中 reg_spill_count 仅表示 regalloc 路径的精确溢出数（alloc._spill_slots 长度）。
• LLVM 侧溢出统计使用 llvm_spill_slots，不要混入 reg_spill_count。
• 若 CNN benchmark 的 regalloc 路径溢出为 0，则 reg_spill_count 应为 0；LLVM 近似溢出是另一指标，不应出现在通用键中。

🔴 LLVM 溢出统计 5 节：sw/lw 匹配 4 字节可能漏掉 8 字节 spill

RV64 下 sd/ld 也用于 spill（8 字节），但正则只统计 sw/lw/fsw/flw，且仅 4 字节。若 LLVM 使用 sd 做 spill（常见），溢出计数会严重偏低。建议：增加对 sd <reg>, N(sp) 的统计（区分帧保存和溢出），或明确说明该近似仅适用 4 字节 spill 场景，并评估偏差。

🟡 §3.3 CNN 参数表：物理寄存器 _INT_REGS（28 个）但未定义

_INT_REGS 是内部常量，文档未说明其内容（如是否包含 ra/sp/gp/tp）。线性扫描分配器通常需要排除固定寄存器，若包含了保留寄存器，可能导致分配错误。建议要么在文档中列出实际寄存器集合，要么引用 bench_utils.py 中的定义。

🟡 §7 使用示例：--repeats 30 但 JSON 输出示例中 "repeats": 3

不一致。示例 run_benchmark.py 使用 --repeats 30，但 §6.2 JSON 样例如 "repeats": 3。要么统一数字，要么说明 repeats 可变。

💭 §3.1/3.2 参数表格式不统一

§3.1 表格含空行，§3.2 表格后有两个空行，§3.3 表格后无空行。建议统一空行数，保持整洁。

💭 §6.3 汇总表：LLVM 列标题 LLVM-FD 但前面用 llvm_fd_instrs

表头 LLVM-FD 与键名 llvm_fd_instrs 不一致，可能引起误解。建议表头直接用 LLVM RV64FD 或 LLVM-FD Instrs。

💭 §8 TODO 第 3 点：与LLVM后端对比不公平

已知不公平，但文档前文已说明（§3.3 注意）。建议将 TODO 改为更具体的工作项，如“完善指令选择以支持更多算子，使对比更公平”，而非简单陈述不公平。

💭 §4.2 sv_static_instrs 值“~46”与 §6.2 JSON 示例 “46” 不一致

§3.3 表格中 ScratchV 输出“~57 条伪指令”，§4.2 又说“~46”。建议统一数字，或明确区分“伪指令”与“静态指令”计数，并确保示例 JSON 采用合理值。

---