Impl/topic16

[FeelTheBeats]

No description provided.

[github-actions]

🤖 AI Code Review

共审查 3 个变更文件

📁 .github/workflows/ci.yml

Code Review: .github/workflows/ci.yml

🔴 Missing LLVM installation — The comment says "llvm-as/opt" are invoked, but no apt install llvm or equivalent step is visible in this diff. If LLVM isn't already in the job's setup, both the test and benchmark steps will hard-fail. Verify the job has a prior step like sudo apt-get install -y llvm clang or a container image that includes it.

🟡 No timeout on LLVM steps — LLVM codegen tests can hang on compiler bugs. Add timeout-minutes to both the test step and benchmark step (e.g., timeout-minutes: 5) to prevent runaway CI minutes.

🟡 Report step should use continue-on-error — Line ~230: The benchmark report generation is non-critical infrastructure. If the script fails, it shouldn't block the pipeline:

- name: Topic 16 LLVM codegen case report
  continue-on-error: true
  run: |

💭 File existence guard in summary — Good pattern with if [ -f ... ]. Consistent with existing steps.

💭 Test file dependency — If tests/test_llvm_codegen_topic16.py is introduced in this same PR, consider making the workflow step conditional or confirming both land in the same commit to avoid a broken CI window.

---

📁 benchmarks/cases/topic16_llvm_feature.dsl

🟡 Implicit inputs may diverge from other cases — Lines 12–20: x, a, b, m1, m2, v, z are used without any explicit declaration. If other .dsl cases in benchmarks/cases/ use a input/def header, this file will fail or be inconsistent. Verify the DSL frontend's implicit-binding behavior handles bare identifiers reliably, and add declarations if the convention differs.

🟡 No metadata for the harness — The comment says "the case report asserts those markers" and mentions backend="llvm", but nothing in the file machine-readably specifies the backend or the expected markers (e.g. // backend: llvm, // expect: getelementptr, fmul, fadd, tanhf). If the benchmark runner discovers these from the file path or a separate manifest, fine; otherwise this coupling is implicit and fragile.

💭 Comment says "five NN operators" but lists the adds too — The operator chain is relu → dot → matmul → softmax → gelu → add×4. The comment is technically accurate about the NN operators but the total chain has 9 ops. Minor; just ensure the case report names match the count it reports.

---

📁 docs/topics/16-LLVM代码生成-开发文档.md

🔴 Bug: _dim_of 负轴越界检查错误 — §3.2：len(shape) > abs(idx) 对 axis=-1 且 1D tensor 返回 False（1 > 1），softmax n = _dim_of(..., axis=-1) 在 1D 输入时恒返回 default=1，数值必然错。
Suggestion: 先归一化 idx：if idx < 0: idx += len(shape)，再检查 0 <= idx < len(shape。

🔴 Bug: _emit_br_if 无边界保护 — §2.6：直接 targets[0] / targets[1]，若 instr.target 为空串或缺第二个目标，抛 IndexError 而非 LLVMCodegenError。
Suggestion: 校验 len(targets) >= 1，缺失时抛明确异常。

🟡 Signature 不一致 — §1.2.6 表格写 _emit_binary(instr, op: str)，§2.3 代码写 (instr, fop, iop=None)。实现者看表格会漏掉 iop 参数。
Suggestion: 表格签名改为 (instr, fop: str, iop: str | None = None)。

🟡 _coerce_operand 类型覆盖不足 — §2.3 仅处理 i32↔float/double，else: raise。若未来出现 i64→float 或 i16→float 静默崩溃。
Suggestion: 文档注明当前范围，或至少支持 i64↔double。

🟡 _float_literal 丢负零 — §2.3：-0.0 == 0.0 在 Python 为 True，负零被写为 "0.0" 而非 "-0.0"。绝大多数场景无害，但 softmax max 初始化用 -3.4e38 无此问题，仅记录。

🟡 _alloc_slot prologue 与 entry 块关系未明确 — §2.1：说 prologue 指令"自动属于 entry 块"，但 LLVM 要求 entry 块中不能有 alloca 以外的非声明指令出现在 br 之前（除 alloca/call 等允许在函数开头的指令）。alloca 在 LLVM IR 中可以出现在 entry block 中的 br 之前，但如果有 store 等指令则不合法。文档未约束 prologue 中允许哪些指令。
Suggestion: 明确 _prologue 只允许 alloca（LLVM 10 语义），store 等须在 br 之后的块中发射。

🟡 _emit_map 和 _require_elements 仅在偏差部分出现 — §"修复轮补丁要点"引入 _emit_map、_require_elements、_validate_return_types、_emit_unsupported、_check_unbounded_loops，但 §1.2.6 的 helper 表格未收录。实现者读正文找不到这些函数的契约定义。
Suggestion: 在 §1.2.6 表格中补充这些函数，或标注为"见修复轮补丁"。

🟡 _loop_open 中 IV bind 位置 vs. exit 块使用 — §2.5：self._bind(ir_name, iv, "i32") 在 body 块执行，但 _named_values 是全局状态，循环后引用仍能解析。文档 §2.5 边界说明提到"header 的 load 仍支配 exit"，但 iv SSA 名是 header 中定义，bind 在 body 中只是映射关系——语义上成立，但实现者可能误以为 body 没执行时（空循环/条件恒假）绑定不生效。
Suggestion: 将 bind 移至 header 中 iv 定义之后、body 之前。

💭 测试文件命名偏差未在前文声明 — §4.1 说新增 test_llvm_codegen_llvm_tools.py，实现结果说实际是 test_llvm_codegen_topic16.py。偏差部分已记录，但 §4.1 作为规范文档，建议加 > 注脚标注实际命名。

💭 _float_to_llvm_hex 的 pack/unpack 链 — struct.pack("<f", value) 截断为 f32，struct.pack("<d", float(f32)) 再展开为 f64 位型。逻辑正确，但对极端值（inf/nan）0x%016X 输出的是 f64 布局的 inf/nan 位模式，与 LLVM 预期一致。仅记录为已验证路径。

---