Skip to content

feat: add Topic 17 v1.3 — linear scan register allocator, bottleneck scenarios, dev & design docs - #34

Closed
Cynthia-Xichen wants to merge 12 commits into
ScratchV-Compiler:mainfrom
Cynthia-Xichen:feat/topic17-v1.3-submit
Closed

feat: add Topic 17 v1.3 — linear scan register allocator, bottleneck scenarios, dev & design docs#34
Cynthia-Xichen wants to merge 12 commits into
ScratchV-Compiler:mainfrom
Cynthia-Xichen:feat/topic17-v1.3-submit

Conversation

@Cynthia-Xichen

Copy link
Copy Markdown

Summary

Adds Topic 17 v1.3 deliverables (4 new files, no existing files modified):

  • regalloc_linear_v1.3.py — Linear scan register allocator v1.3 with bugfixes and improvements
  • topic17_bottleneck_scenarios_v1.3.py — Systematic bottleneck scenario testing across 6 dimensions
  • topic17_v1.3开发文档.md — Development documentation
  • topic17_v1.3设计文档.md — Design documentation

Xi and others added 12 commits July 27, 2026 16:15
- restructured compute_live_intervals to single-pass O(N+V) scan
- fixed self-spill handling: proper SPILL_ prefix, no spurious sw
- fixed spill code insertion order (sw at correct instruction position)
- fixed peak_active tracking position (after allocation, not before)
- added allocator report: peak_active, phys_regs_used, spill details
- comprehensive 9-scenario benchmark with v1.0 vs v1.2 comparison
- added regalloc_scenario_analysis_report_v1.2.md
Full scenario-level analysis covering 64 test cases across two rounds:
- 9 high-dimension scenarios (dense_60, spiral_60, large_800x150, etc.)
- 32 general edge-case scenarios (T01-T32)
- 23 core allocation semantic scenarios (C01-C23)

Key findings: 2 correctness bugs (alloc_map eviction, peak_active cap),
4 optimization directions, detailed per-scenario metrics.
Add 23 systematic test scenarios across 6 bottleneck dimensions:
- Activity (4): slight/moderate/severe/zero overlap
- Lifetime (4): short-lived/long chain/mixed/alternating
- Reuse (4): single-spill-many-uses/many-spills-one-use/loop-body/adjacent
- Eviction (4): same-end/skewed/chain/spiral
- Stack pressure (3): wave/cascade/zero-reuse
- Code expansion (4): max-spill/expansion-ratio/dense-chain/random

Test framework in scratchv/backend/ for reproducible runs.
…pick_scratch cache)

Bug A (critical·correctness): spill() eviction branch omitted
alloc_map update, causing evicted vregs to be read from wrong phys reg
without a reload (lw). Fix: set alloc_map[vreg] = 'SPILL_{vreg}' after
active.pop() in spill().

Bug B (medium·diagnostic): peak_active capped by pool_size because
self-spilled intervals are not in active[]. Fix: add peak_real_pressure
field that counts self-spilled intervals too.

Bug C (low·code quality): _pick_scratch picked a different scratch reg
each time for the same spilled vreg. Fix: add _scratch_cache with
vreg_hint parameter for last-used-register memory.

Also: fix eviction/self-spill counting in test framework (Bug A fix
changed alloc_map semantics, count evictions from spill_code entries).
+ add code comments
+ sync topic17 design doc and dev doc to v1.3
- restructured compute_live_intervals to single-pass O(N+V) scan
- fixed self-spill handling: proper SPILL_ prefix, no spurious sw
- fixed spill code insertion order (sw at correct instruction position)
- fixed peak_active tracking position (after allocation, not before)
- added allocator report: peak_active, phys_regs_used, spill details
- comprehensive 9-scenario benchmark with v1.0 vs v1.2 comparison
- added regalloc_scenario_analysis_report_v1.2.md
Full scenario-level analysis covering 64 test cases across two rounds:
- 9 high-dimension scenarios (dense_60, spiral_60, large_800x150, etc.)
- 32 general edge-case scenarios (T01-T32)
- 23 core allocation semantic scenarios (C01-C23)

Key findings: 2 correctness bugs (alloc_map eviction, peak_active cap),
4 optimization directions, detailed per-scenario metrics.
Add 23 systematic test scenarios across 6 bottleneck dimensions:
- Activity (4): slight/moderate/severe/zero overlap
- Lifetime (4): short-lived/long chain/mixed/alternating
- Reuse (4): single-spill-many-uses/many-spills-one-use/loop-body/adjacent
- Eviction (4): same-end/skewed/chain/spiral
- Stack pressure (3): wave/cascade/zero-reuse
- Code expansion (4): max-spill/expansion-ratio/dense-chain/random

Test framework in scratchv/backend/ for reproducible runs.
…pick_scratch cache)

Bug A (critical·correctness): spill() eviction branch omitted
alloc_map update, causing evicted vregs to be read from wrong phys reg
without a reload (lw). Fix: set alloc_map[vreg] = 'SPILL_{vreg}' after
active.pop() in spill().

Bug B (medium·diagnostic): peak_active capped by pool_size because
self-spilled intervals are not in active[]. Fix: add peak_real_pressure
field that counts self-spilled intervals too.

Bug C (low·code quality): _pick_scratch picked a different scratch reg
each time for the same spilled vreg. Fix: add _scratch_cache with
vreg_hint parameter for last-used-register memory.

Also: fix eviction/self-spill counting in test framework (Bug A fix
changed alloc_map semantics, count evictions from spill_code entries).
+ add code comments
+ sync topic17 design doc and dev doc to v1.3
topic17: v1.3 bugfix (alloc_map eviction, peak pressure, scratch cache)
@github-actions

Copy link
Copy Markdown

🤖 AI Code Review

共审查 10 个变更文件
⚠️ 另有 10 个文件超过上限(最多 10 个)未审查

📁 .github/prompts/code-review-prompt.md

🟡 移除文件可能导致下游断裂 — 该文件 code-review-prompt.md 定义了 Code Reviewer Agent 的系统提示。若被其他流程(如 CI/copilot-instructions 或脚本引用)显式引入,删除后 agent 将丢失行为规范,导致审查质量下降。建议确认所有引用点已更新或迁移,否则保留文件或至少提供迁移说明。

🟡 缺少替代方案说明 — 如果该提示已内联到其他位置(如代码库根目录的 .github/copilot-instructions.md),应在 commit message 或相关文档中明确新位置,避免后续维护者困惑。

💭 考虑归档而非删除 — 若该提示不再使用,可移至 _archived/ 或添加 README 说明废弃原因,帮助历史追溯。


📁 .github/workflows/ai-code-review.yml

🔴 安全风险:pull_request_target 事件 — 使用 pull_request_target 可让外部 PR 获得 secrets 访问权。虽然此处仅用 head.sha checkout 且不执行构建,但若攻击者能控制 API 调用内容(如 USER_MSG 注入),仍可能造成风险。建议:优先使用 pull_request 事件,若必须用 pull_request_target,则严格限制 API 调用参数(如通过 jq 构建已做到),并考虑添加 if: github.event.pull_request.head.repo.full_name == github.repository 防止 fork 触发。

🔴 Bug:git diff 基准可能不准确 — Line 17: git diff --name-only "origin/$BASE_REF...HEAD" 依赖远程 origin/main 的最新状态,但 PR 的 base SHA 可能不是 origin/main 的 HEAD,导致漏掉或误判变更。建议:使用 ${{ github.event.pull_request.base.sha }}${{ github.event.pull_request.base.ref }} 并确保 fetch 该 ref。

🟡 curl 响应解析脆弱 — Line 74-77: 使用 -w "\n%{http_code}" 并用 tail -1 / sed '$d' 分离 body 和 HTTP 码。若 body 末尾恰好包含数字行(如 200),会误判为 HTTP 状态码。建议:改用 -o body.txt -w "%{http_code}" 写文件,或使用 --output--write-out 分离输出。

🟡 速率限制缺失处理 — 只有 1 秒固定间隔,未处理 API 返回 429 或 5xx 重试后的背退。建议:在 curl 重试中增加指数退避,或检查响应头中的 Retry-After

🟡 评论匹配条件脆弱 — Line 127: c.body.startsWith('# 🤖 AI Code Review') 可能因评论截断、换行或加粗而匹配失败。建议:在评论中嵌入一个隐藏的 HTML 注释(如 <!-- review-id: xxx -->)作为唯一标识符用于查找。

💭 脚本过长,建议提取为 Action — 整段逻辑超过 250 行,内联在 workflow 中难以维护和测试。建议将主要逻辑抽取为独立 GitHub Action 或脚本文件,并添加单元测试。

💭 FILE_COUNT 递增位置 — 成功审查后递增,但 MAX_REVIEW_FILES=10 时实际最多处理 10 个文件(FILE_COUNT 从 0 到 9),符合预期。但若 MAX_REVIEW_FILES 改为 0,会错误处理所有文件(因为 0 >= 0 为 true,导致跳过所有)。建议在循环头部先检查 if [ "${FILE_COUNT}" -ge "${MAX_REVIEW_FILES}" ] && [ "${MAX_REVIEW_FILES}" -gt 0 ]


📁 .github/workflows/ci.yml

🔴 安全风险:自托管 runner 上的工作区操作 — 同步步骤 rm -rf "$WORKSPACE"cp -a "$MIRROR" "$WORKSPACE" 在 PR 触发时执行,若 PR 修改了 $MIRROR 路径或通过符号链接攻击,可能导致非预期文件删除或恶意代码注入。建议使用 actions/checkout 并严格限制工作区。

🔴 重复代码:同步步骤在 test 和 benchmark job 中完全复制 — 两段 20+ 行的 shell 脚本完全一致,违反 DRY 原则,维护困难。建议提取为 composite action 或通过 workflow 的 workflow_call 复用。

🟡 静默吞没错误 — 多个步骤(如 llvm_cache_compare.pydashboard.pysplit_cnn_to_single_ops.py)使用 || echo "WARNING: ..." 级别处理,导致失败不被检测。后续步骤可能依赖这些命令的输出文件,若缺失则 Pages 部署内容不完整。建议使用 set -e 或显式检查文件存在性。

🟡 非标准环境变量FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true 不是 GitHub 官方支持的变量,可能引发 action 兼容性问题或行为异常。建议移除或确认其必要性。

🟡 忽略测试文件无说明pytest 命令中 --ignore=tests/test_simulator.py 没有注释原因,其他开发者难以理解为什么跳过该测试。建议添加注释说明。

🟡 pip 升级可能破坏环境 — 每次运行 pip install --upgrade pip 可能引入不兼容版本,且自托管 runner 环境可能已固定 pip 版本。建议移除或使用 pip install --upgrade pip setuptools wheel 并锁定版本。

🟡 作业摘要步骤无错误处理cat benchmark_reports/github_summary.mdtail -20 benchmark_reports/llvm_vs_scratchv.md 未检查文件是否存在,若缺失则步骤失败。建议添加 if [ -f ... ] 判断。

💭 重复创建目录 — 多个步骤中重复 mkdir -p benchmark_reports,建议合并到第一个步骤或顶部。

💭 浅克隆与完整 fetch 混合 — 使用 --depth=1 但后续 git fetch origin main 可能仍需要完整历史,建议直接 git fetch --unshallow 或移除 --depth=1


📁 .gitignore

🔴 Blocker: Deletion of entire .gitignore — Removing the .gitignore file will cause build artifacts (.o, .pyc, venv/, models/, output binaries, etc.) to be tracked by version control. This bloats the repository, risks accidental commits of large or sensitive files, and breaks team conventions. If intentional, please provide an alternative mechanism (e.g., global gitignore, policy) and document the rationale. Otherwise, restore the file.


📁 docs/regalloc_scenario_analysis_report_v1.2.md

🔴 数据符号错误 — 表格中 dense_60 场景的 sw减少 列值为 -34,但实际 v1.0 sw=34、v1.2 sw=0,减少量应为 +34(或 -34 表示增加了?)。建议统一为正值(v1.0 - v1.2)或明确说明符号含义。

🟡 笔误 — 第 3 节“关键发现”中写 slarge_800x150,应为 large_800x150(多了一个 s)。

🟡 优化方向 4 描述模糊 — “省一次寄存器传输”不准确:改进是优先复用上次的物理寄存器,省的是 lwmv 指令,不是“寄存器传输”。建议改为“省一次 lw 或 mv 操作”。

💭 表格一致性spiral_60 场景 v1.2 peak=36,超过物理寄存器数 27,而标题是“peak_active”,但未在表头或注释中说明该指标是虚拟寄存器活跃数(非物理寄存器占用量)。建议在表头或脚注注明。


📁 docs/regalloc_scenario_analysis_report_v2.md

🔴 数据不一致:T08_short_alternating 场景出现两次且 eviction 次数矛盾

  • 第一次出现(“第二轮测试”部分,表后描述)记 eviction = 1,第二次出现(“边界场景验证结果”前)记 eviction = 33。两个表格的 peak_active 和自溢数相同(27/54),但 eviction 相差 32 次。
  • 影响:读者无法判断正确值,严重损害报告可信度。
  • 建议:核查原始测试数据,保留一个准确版本,并说明为何出现差异(如不同种子或 pool 配置)。

🟡 拼写错误:burts_64 → bursts_64(场景 4-5 表格前列表)。

🟡 “问题”列表中的表格格式:在 T08 第一次出现后的“问题”列表使用了类似 Markdown 表格但未对齐,可能影响阅读。建议改为普通列表或正确对齐的表格。

💭 场景 4-5 表格中缺少 conv2d_30 和 butterfly_75 的说明:这两个场景出现在表格前列表中,但表格后未单独解释,读者可能不清楚其构造方式。建议在表格前或后补充一句话说明。

💭 Bug A 修复方案中 SPILL_ 前缀格式:示例使用 f"SPILL_{spill_interval.vreg}",但原文其他处使用 SPILL_ + 数字编号(如 SPILL_0),建议统一约定(如 SPILL_v0SPILL_0),避免混淆。


📁 docs/topic17_bottleneck_report.md

🔴 STORE_ONLY 模式未标注 — 场景 F02 (sw=2, lw=0), C01 (sw=1, lw=0), C04 (sw=2, lw=0) 等满足 STORE_ONLY 条件(有 sw 无 lw),但报告中未按字段说明标注。这可能暗示分配器在 eviction 后缺失 reload,是需要重点关注的 bug,应在数据旁或表格中明确标记。

🔴 LOADS>STORES 模式未标注 — 场景 F03 (sw=35, lw=6) 和 F04 (sw=137, lw=68) 满足 lw > sw 比例异常,但未标注。根据字段说明,这表示同一定义被多次 reload,可能反映分配器复用策略效率问题,应补充分析。

🟡 POOL 变量未定义 — 构建代码段中多处使用 POOL 变量,但读者可能不清楚其值(应为 27)。建议在文档开头或每个场景前明确 POOL=27,确保可复现性。

🟡 重复字段造成混淆 — 字段说明中同时列出 lines/asm_linesbytes/asm_bytes,但报告中仅使用前一组。建议统一命名,避免读者困惑。

🟡 缺少异常标志列 — 汇总表未包含 STORE_ONLYLOADS>STORESSLOT_NO_REUSEPEAK_LOCKED 等标志列,不利于快速定位潜在问题。建议在汇总表中增加这些列或注释。

💭 临时路径硬编码 — 报告末尾提到测试框架在 /tmp/topic17_bottleneck_scenarios.py,该路径可能不可持久。建议改为说明如何从仓库获取或提供生成脚本的链接。

💭 avg_uses 的精度 — 部分场景 avg_uses 显示为 0.97 或 0.98,但根据原始数据计算可能精确到 0.9697,建议保留两位小数时统一四舍五入规则,避免微小误差积累。


📁 docs/topic17_v1.3开发文档.md

🔴 严重问题:可用物理寄存器数量描述不准确
文档说“RISC-V x1-x31 中可用的 27 个”,但实际 ABI 会保留 x1(ra)、x2(sp)、x3(gp)、x4(tp),若实现为完全通用分配器,则可用数可能仅为 22~23 个。建议明确列出被保留的寄存器列表,并说明是否考虑 callee‑saved 寄存器。若未处理,可能导致生成的汇编在调用点破坏调用者保存的寄存器。

🔴 严重问题:算法描述未反映 v1.3 修复
2.2 节核心算法仍停留在 v1.0 的“溢出 end 最晚”策略,未集成 v1.1 的自溢处理、v1.3 的 alloc_map eviction 更新等内容。读者若仅读算法描述,会得到错误的行为预期。建议将最新修正后的完整流程(包括 _pick_scratch 缓存、self‑spill 标记、spill code 插入位置)直接写进 2.2 节,附录仅保留历史变更记录。

🟡 建议:缺少物理寄存器保留列表的正式声明
文档仅在 3.3 边界条件中提及“x0 和 ra 被正确保留”,但未明确 sp、gp、tp 等是否保留。若分配器意外将 sp 分配给 vreg,将导致栈操作崩溃。建议在 2.2 节增加一个固定列表 reserved_regs = {"x0", "x1", "x2", "x3", "x4"} 并在分配时跳过。

🟡 建议:测试验收标准中的 check list 未明确状态
4.3 节列出的 6 项验收条件均为未勾选([ ]),但 6. 开发进度中“自测与调试”显示“344/344 通过,4 跳过”,暗示大部分已满足。请将已完成的条目改为 [x],并注明哪些条件尚未达成(如“生成的汇编中不存在任何 vreg 引用”是否已验证?)。

🟡 建议:风险项“栈槽分配冲突”与未来优化方向矛盾
5. 节缓解措施称“每个 vreg 独占一个槽位”,但 7.6 节 Opt2 提出“栈槽复用(slot reuse)”。当前设计若采用独占,则栈槽复用优化将与其冲突。建议在风险项中说明当前策略为“每个 vreg 独立槽位,未复用”,并标注 Opt2 为未来优化方向,避免误导。

💭 小问题:示例代码中 operands 与 defines/uses 重复
2.1 节示例 LsInstruction(0, "add", ["v1", "v2", "v3"], defines={"v1"}, uses={"v2", "v3"}) 中 operands 列表与 defines/uses 集合本质重复。建议明确 operands 是完整指令操作数列表(包括寻址模式等),defines/uses 仅用于活跃分析,可简化成 operands=["v1", "v2", "v3"] 且不必与 defines/uses 完全一致,避免冗余输入。

💭 小问题:术语不一致
文档同时使用“spill”和“溢出”,如“spill code” vs “溢出策略”;“active list” vs “活跃列表”。建议全篇统一为英文术语(如 “spill code”、“active list”),或统一为中文,避免混淆。


📁 docs/topic17_v1.3设计文档.md

🔴 严重Bug:自身溢出变量的定义点缺少存储 — 文档2.4节和Step4中,get_allocated_code 仅对 uses 中的SPILL_变量插入了lw,但 defines 中的SPILL_变量未分配临时寄存器,直接使用 "SPILL_vreg" 作为操作数,导致无效汇编。定义后也需立即sw存储,但代码中 if d in self._spill_slots and str(rename.get(d,"")) in self.phys_regs 排除了SPILL_变量,导致值丢失。修复:定义时也需通过 _pick_scratch 分配临时寄存器,并在定义后插入sw。

🔴 严重Bug:栈槽偏移方向错误 — 2.3节和Step4中,offset = next_stack_offset 并从0递增,但RISC-V栈向下生长,offset(sp) 访问sp上方(非栈区)。应使用负偏移,如 sw xxx, -offset(sp),或先调整sp再使用正偏移。当前设计会导致溢出变量写入非法内存。

🟡 寄存器冲突风险:_pick_scratch 缺乏与active区间的隔离 — 2.4节称 _pick_scratch 仅避免与当前指令操作数冲突,但未考虑当前活跃的物理寄存器(active列表中的寄存器)。若选中的scratch寄存器被其他活跃区间占用,则临时加载会覆盖正确值。需在指令点维护活跃寄存器集合,并从中排除。

🟡 示例场景错误 — 5.4节Bug A示例中,v1=[1,3),v2=[3,5),v1的end=3 ≤ v2的start=3,v1已过期,a1被释放,v2应分配到a1,无需evict v0。此场景无法复现bug,需调整v1的end值(如end=4)以触发溢出。

🟡 验收标准表述不准确 — 4.3节“分配前后的二进制输出一致”应为“分配前后的程序执行结果一致”,寄存器分配会改变指令编码,二进制不可能相同。

💭 free_regs初始化矛盾 — 2.2节初始化为 [x1...x31],但2.6节说明排除ra、sp等后仅27个可用。应统一为27个可分配寄存器列表。

💭 active列表未明确维护顺序 — 2.2节要求按end升序维护,但Step3代码仅 active.append,未排序。选择victim时需遍历找最大end,虽然可行,但若改为堆/有序插入可提升性能。建议明确实现方式。

💭 历史修复信息干扰可读性 — 5.2-5.4节包含大量v1.1和v1.3修复历程,作为设计文档可精简为单向描述,避免读者混淆。

💭 测试覆盖不足 — 4.1节缺少对自身溢出、_pick_scratch 冲突、栈槽偏移边界、跨指令多次使用溢出变量等场景的测试。

💭 栈槽偏移表述不清 — 2.3节“偏移从0开始向下增长”应明确“向下增长”指负偏移,当前写法易误解为正偏移。


📁 docs/topic17开发文档.md

🔴 文档与实际代码不一致 — 2.2 节数据结构变更未列出 _scratch_cachepeak_real_pressure,但 7.5 节已添加。建议同步更新内部设计部分。

🔴 3.3 节边界条件未确认 — 5 个 checkboxes 均未勾选,文档未说明是否已处理,导致读者不确定 x0/ra 保留等逻辑是否已实现。

🔴 7.4/7.5 指标未在 report() 示例中体现 — 7.4 展示了 peak_active 输出,但 7.5 新增的 peak_real_pressure 未更新示例,导致用户查阅时缺少预期输出。

🟡 测试状态与修复记录矛盾 — 6 节自测状态为“344/344 通过,4 跳过”,但 7.5 节声称已修复 3 个 bug,测试通过数应增加。建议更新进度或说明修复后测试结果。

🟡 7.2 用例3 依赖 Spike 仿真器 — 未说明 Spike 是否已集成、如何验证,依赖缺失可能导致验收无法重复。

🟡 7.6 优化方向缺乏优先级和计划 — 列出了 4 个优化方向但无时间安排或决策依据,影响后续开发排期。

🟡 7.5 Bug A 验证描述可能误导 — “stores == loads,全部消除”暗示溢出完全消失,但实际应理解为每次 spill 后都有对应 reload,建议澄清。

🟡 2.2 节状态管理遗漏字段_spills_stack_slots_next_stack_offset 列了,但未包含 _scratch_cachepeak_activepeak_real_pressure,应补充。

💭 7.3 优化1 复杂度标注不精确 — “O(N+V)” 应为 O(N+V) 但 N 和 V 非独立,可简化为 O(N) 或 O(N+V) 并说明假设。

💭 7.3 优化2 中 phys_regs[0] 未定义 — 读者不知 phys_regs 是什么,可改为“物理寄存器池中第一个寄存器”。

💭 7.3 优化3 未提 lw 插入位置 — 只提到 sw 按指令位置插入,lw 的插入时机(使用前)未说明。

💭 7.5 Bug B 修复中 peak_real_pressure 统计时机未说明 — 是否在 active.append 之后统计?应与 peak_active 统计时机统一明确。

💭 7.6 Opt 1/3 预期效果缺乏数据来源 — 5-15%、37% 等数字无参考文献或实验支撑,建议标注为估算或补充测量方法。



⚠️ 未审查的文件

  • docs/topic17设计文档.md
  • scratchv/backend/regalloc_linear.py
  • scratchv/backend/regalloc_linear_v1.3.py
  • scratchv/backend/topic17_bottleneck_scenarios.py
  • scratchv/backend/topic17_bottleneck_scenarios_v1.3.py
  • tests/stress/reg_pressure_32.dsl
  • tests/stress/reg_pressure_32.meta.json
  • tests/stress/reg_pressure_loop.dsl
  • tests/stress/reg_pressure_loop.meta.json
  • tests/test_regalloc_linear.py

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看上去删了很多不该删的文件呢, 建议先和origin/main对齐

@Cynthia-Xichen Cynthia-Xichen closed this by deleting the head repository Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants