From 771bf6b0d15438652a6f71416f2ed62e733b75e4 Mon Sep 17 00:00:00 2001 From: nyash-codex Date: Mon, 15 Dec 2025 06:00:48 +0900 Subject: [PATCH] feat(joinir): Phase 132-P3 - Exit PHI collision early detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added verify_exit_phi_no_collision() to contract_checks.rs for early detection of ValueId collisions between exit PHIs and other instructions. Problem detected: - If exit_phi_builder uses builder.value_gen.next() (module-level) instead of func.next_value_id() (function-level), ValueIds can collide: Example: - bb0: %1 = const 0 (counter init) - bb3: %1 = phi ... (exit PHI - collision!) Previous behavior: - Error only detected at LLVM backend runtime - Cryptic error: "Cannot overwrite PHI dst=1" New behavior: - Panic at Rust compile time (debug build) - Clear error message with fix suggestion: "Exit PHI dst %1 collides with instruction in block 0 Fix: Use func.next_value_id() in exit_phi_builder.rs" Benefits: - 🔥 Fail-Fast: Catch errors during Rust compilation, not LLVM execution - 📋 Clear messages: Exact collision point + fix suggestion - 🧪 Testable: verify_exit_phi_no_collision() can be unit tested 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- CURRENT_TASK.md | 6 +- docs/development/current/main/10-Now.md | 6 + .../phase132-case-c-llvm-exe.md | 56 ++++++-- .../main/phase131-11-case-c-summary.md | 3 +- .../phase131-3-llvm-lowering-inventory.md | 2 +- .../current/main/phases/phase-132/README.md | 29 +++- .../joinir/merge/contract_checks.rs | 125 ++++++++++++++++++ 7 files changed, 211 insertions(+), 16 deletions(-) diff --git a/CURRENT_TASK.md b/CURRENT_TASK.md index 99db38ed..9a7a48ba 100644 --- a/CURRENT_TASK.md +++ b/CURRENT_TASK.md @@ -18,10 +18,11 @@ - debug flag SSOT / DebugOutputBox 移行 / error tags 集約 / carrier init builder まで整備済み。 - **LLVM exe line SSOT 確立**: `tools/build_llvm.sh` を使用した .hako → executable パイプライン標準化完了。 - **LLVM AOT（Python llvmlite）ループ復旧**: `apps/tests/loop_min_while.hako` が EMIT/LINK/RUN まで到達（Phase 131-3..10）。 - - Case C（`loop(true)` + break/continue）は Phase 131-11 で Pattern/shape guard の基盤まで到達。残りは Case C の parity/仕上げ（SSOT を見ながら詰める）。 -- **Phase 132 完了**: Pattern 1（Simple While）の `return i` が VM/LLVM で一致（3 を返す）まで根治。 + - Case C（`loop(true)` + break/continue）は Phase 131-11 で Pattern/shape guard を整備し、Phase 132-P2 で LLVM EXE まで parity（`Result: 3`）を確認。 +- **Phase 132 完了**: ループ exit 値（exit PHI / boundary）の VM/LLVM parity を根治。 - JoinIR/Boundary: exit 値を境界に明示的に渡す - LLVM Python: PHI を落とす/上書きする経路を除去（PHI SSOT を保護） + - JoinIR merge: exit PHI dst の allocator を function-level に統一（ValueId 衝突を排除） - **Phase 88 完了**: continue + 可変ステップ（i=i+const 差分）を dev-only fixture で固定、StepCalculator Box 抽出。 - **Phase 89 完了**: P0（ContinueReturn detector）+ P1（lowering 実装）完了。 - **Phase 90 完了**: ParseStringComposite + `Null` literal + ContinueReturn（同一値の複数 return-if）を dev-only fixture で固定。 @@ -40,6 +41,7 @@ - `docs/development/current/main/phase131-5-taglink-fix-summary.md` - `docs/development/current/main/phases/phase-132/README.md` - `docs/development/current/main/investigations/phase132-llvm-exit-phi-wrong-result.md` +- `docs/development/current/main/investigations/phase132-case-c-llvm-exe.md` --- diff --git a/docs/development/current/main/10-Now.md b/docs/development/current/main/10-Now.md index 258adca5..12ac1a05 100644 --- a/docs/development/current/main/10-Now.md +++ b/docs/development/current/main/10-Now.md @@ -9,6 +9,12 @@ - 結果: `/tmp/p1_return_i.hako` が 3 を返す（VM 一致） - 詳細: `investigations/phase132-llvm-exit-phi-wrong-result.md` +**追加（Phase 132-P2）: Case C の Exit PHI ValueId 衝突を修正** +- 原因: `exit_phi_builder.rs` が module-level allocator を使い、同一関数内で ValueId が衝突し得た +- 修正: `func.next_value_id()`（function-level）へ統一（`bd07b7f4`） +- 結果: `apps/tests/llvm_stage3_loop_only.hako` が LLVM EXE でも `Result: 3`（VM 一致） +- 詳細: `investigations/phase132-case-c-llvm-exe.md` + ## 2025‑12‑14：現状サマリ （補足）docs が増えて迷子になったときの「置き場所ルール（SSOT）」: diff --git a/docs/development/current/main/investigations/phase132-case-c-llvm-exe.md b/docs/development/current/main/investigations/phase132-case-c-llvm-exe.md index 255f27a4..df00d999 100644 --- a/docs/development/current/main/investigations/phase132-case-c-llvm-exe.md +++ b/docs/development/current/main/investigations/phase132-case-c-llvm-exe.md @@ -4,7 +4,7 @@ 2025-12-15 ## Status -🔴 **FAILED** - LLVM executable returns wrong result +✅ **FIXED** - VM / LLVM EXE parity achieved ## Summary Testing `apps/tests/llvm_stage3_loop_only.hako` (Pattern 5: InfiniteEarlyExit) in LLVM EXE mode reveals a critical exit PHI usage bug. @@ -32,7 +32,39 @@ static box Main { ## Actual Behavior - **VM execution**: `Result: 3` ✅ -- **LLVM EXE**: `Result: 0` ❌ +- **LLVM EXE**: `Result: 0` ❌（修正前） + +## Final Root Cause (Confirmed) + +**JoinIR merge 側の Exit PHI ValueId 衝突**。 + +`src/mir/builder/control_flow/joinir/merge/exit_phi_builder.rs` が PHI dst の割り当てに +`builder.value_gen.next()`（モジュールレベルのカウンタ）を使っていたため、 +**同一関数内で `ValueId` が衝突**し、結果として exit 側で参照すべき PHI が別の値に潰れていた。 + +例（概念）: + +- `bb0: %1 = const 0`（counter 初期値） +- `bb3: %1 = phi ...`（exit PHI） + +**同じ `%1`** になり得るため、exit 側で `counter` を読んでも 0 になってしまう。 + +### Fix + +PHI dst の割り当てを **関数ローカルの allocator** に統一する。 + +- Before: `builder.value_gen.next()`（module-level） +- After: `func.next_value_id()`（function-level） + +コミット: +- `bd07b7f4 fix(joinir): Phase 132-P2 - Exit PHI ValueId collision fix` + +### Verification + +| テスト | VM | LLVM | +|---|---:|---:| +| Pattern 1 (`/tmp/p1_return_i.hako`) | 3 ✅ | 3 ✅ | +| Case C (`apps/tests/llvm_stage3_loop_only.hako`) | Result: 3 ✅ | Result: 3 ✅ | ## Root Cause Analysis @@ -52,6 +84,10 @@ bb3: ; Exit block **MIR is correct**: The exit block (bb3) has a PHI node that receives the counter value from bb6, and subsequent instructions correctly use `%1`. +補足: +- 上記の “MIR が正しい” は「構造として PHI がある」意味で、**ValueId 衝突があると意味論が壊れる**。 +- 本件は “JoinIR merge の ValueId allocator の選択” が原因だったため、LLVM 側で 0 に見える形で顕在化した。 + ### LLVM IR (BUG) ```llvm bb3: @@ -68,14 +104,13 @@ bb3: **Bug identified**: The PHI node `%"phi_1"` is created correctly and receives the counter value from `%"add_8"`. However, **all subsequent uses of ValueId(1) are hardcoded to `i64 0` instead of using `%"phi_1"`**. -### Hypothesis -The Python LLVM builder is not correctly resolving ValueId(1) when lowering instructions in bb3. Possible causes: +### Discarded Hypotheses (Superseded) -1. **vmap issue**: The PHI node is created and stored in `self.vmap[1]` during `setup_phi_placeholders`, but when lowering instructions in bb3, `vmap_cur` may not contain the PHI. +当初は「Python LLVM builder が ValueId(1) を解決できず 0 にフォールバックしている」と推測し、 +PHI registration / vmap filtering 周りを疑った。 -2. **Resolution fallback**: When `resolve_i64_strict` fails to find ValueId(1), it falls back to `ir.Constant(i64, 0)`. - -3. **Block-local vmap initialization**: `vmap_cur` is initialized with `dict(builder.vmap)` at the start of each block, but something may be preventing the PHI from being included. +ただし最終的に、本件の主因は **Rust 側（JoinIR merge）の ValueId 衝突**であることが確定したため、 +ここでの Python 側の推測は “症状の説明” に留まる（根因ではない）。 ## Investigation Steps @@ -112,7 +147,7 @@ The Python LLVM builder is not correctly resolving ValueId(1) when lowering inst - Exit PHI generation: `src/mir/join_ir/lowering/simple_while_minimal.rs` - Pattern 5 (InfiniteEarlyExit) lowering -## Root Cause Identified +## Interim Hypothesis (Historical) ### Bug Location `/home/tomoaki/git/hakorune-selfhost/src/llvm_py/llvm_builder.py:342-343` @@ -164,6 +199,5 @@ This ensures PHIs are included in `vmap_cur` when lowering their defining block. - ✅ STRICT mode passes without fallback warnings ## Related Documents -- [Phase 132 Plan](/home/tomoaki/git/hakorune-selfhost/docs/development/current/main/phase132-plan.md) - [Phase 131-3 LLVM Lowering Inventory](/home/tomoaki/git/hakorune-selfhost/docs/development/current/main/phase131-3-llvm-lowering-inventory.md) -- [Simple While Minimal Lowering](/home/tomoaki/git/hakorune-selfhost/src/mir/join_ir/lowering/simple_while_minimal.rs) +- [Phase 132 Summary](/home/tomoaki/git/hakorune-selfhost/docs/development/current/main/phases/phase-132/README.md) diff --git a/docs/development/current/main/phase131-11-case-c-summary.md b/docs/development/current/main/phase131-11-case-c-summary.md index b20efa00..29add77f 100644 --- a/docs/development/current/main/phase131-11-case-c-summary.md +++ b/docs/development/current/main/phase131-11-case-c-summary.md @@ -19,7 +19,8 @@ - PHI/型デバッグ: `docs/reference/environment-variables.md` の `NYASH_PHI_TYPE_DEBUG` / `NYASH_PHI_META_DEBUG` 現状メモ: -- VM では期待値に一致するが、LLVM では結果が一致しないケースが残っている（別トピックとして棚卸し/切り分けが必要）。 +- `apps/tests/llvm_stage3_loop_only.hako` については Phase 132-P2 で VM/LLVM parity（`Result: 3`）まで到達した。 + - 調査ログ: `docs/development/current/main/investigations/phase132-case-c-llvm-exe.md` --- diff --git a/docs/development/current/main/phase131-3-llvm-lowering-inventory.md b/docs/development/current/main/phase131-3-llvm-lowering-inventory.md index 59fbf868..baf30116 100644 --- a/docs/development/current/main/phase131-3-llvm-lowering-inventory.md +++ b/docs/development/current/main/phase131-3-llvm-lowering-inventory.md @@ -10,7 +10,7 @@ | A | `apps/tests/phase87_llvm_exe_min.hako` | ✅ | ✅ | ✅ | **PASS** - Simple return 42, no BoxCall, exit code verified | | B | `apps/tests/loop_min_while.hako` | ✅ | ✅ | ✅ | **PASS** - Loop/PHI path runs end-to-end (Phase 131-10): prints `0,1,2` and exits | | B2 | `/tmp/case_b_simple.hako` | ✅ | ✅ | ✅ | **PASS** - Simple print(42) without loop works | -| C | `apps/tests/llvm_stage3_loop_only.hako` | ✅ | ✅ | ⚠️ | **TAG-RUN** - Loop ok; print/concat path segfaults | +| C | `apps/tests/llvm_stage3_loop_only.hako` | ✅ | ✅ | ✅ | **PASS** - `loop(true)` + break/continue + print/concat works end-to-end (Phase 132-P2) | ## Phase 132 Update (2025-12-15) diff --git a/docs/development/current/main/phases/phase-132/README.md b/docs/development/current/main/phases/phase-132/README.md index 6eeb6f5c..4769f1db 100644 --- a/docs/development/current/main/phases/phase-132/README.md +++ b/docs/development/current/main/phases/phase-132/README.md @@ -2,7 +2,7 @@ **Date**: 2025-12-15 **Status**: ✅ Done -**Scope**: Pattern 1（Simple While）で「ループ終了後の `return i`」が VM/LLVM で一致することを固定する +**Scope**: ループ exit 値（exit PHI / boundary）が VM/LLVM で一致することを固定する（Pattern 1 と Case C を含む） --- @@ -64,3 +64,30 @@ static box Main { 詳細ログ: - `docs/development/current/main/investigations/phase132-llvm-exit-phi-wrong-result.md` +--- + +## 追加: Phase 132-P2（Case C）Exit PHI ValueId collision fix + +Case C（`apps/tests/llvm_stage3_loop_only.hako`）の LLVM EXE 検証で、 +**exit PHI の ValueId 衝突**が原因で `Result: 0` になる問題が見つかった。 + +### Root Cause + +- `src/mir/builder/control_flow/joinir/merge/exit_phi_builder.rs` が + PHI dst の割り当てに `builder.value_gen.next()`（module-level）を使っており、 + 同一関数内で ValueId が衝突し得た。 + +### Fix + +- PHI dst の割り当てを `func.next_value_id()`（function-level）へ統一。 + +コミット: +- `bd07b7f4 fix(joinir): Phase 132-P2 - Exit PHI ValueId collision fix` + +### Verification + +- Pattern 1: VM/LLVM ともに `3` +- Case C: VM/LLVM ともに `Result: 3` + +調査ログ: +- `docs/development/current/main/investigations/phase132-case-c-llvm-exe.md` diff --git a/src/mir/builder/control_flow/joinir/merge/contract_checks.rs b/src/mir/builder/control_flow/joinir/merge/contract_checks.rs index 489ab5ab..fbab0943 100644 --- a/src/mir/builder/control_flow/joinir/merge/contract_checks.rs +++ b/src/mir/builder/control_flow/joinir/merge/contract_checks.rs @@ -96,6 +96,131 @@ pub(super) fn verify_exit_line( } } } + + // Phase 132-P2: Verify exit PHI ValueIds don't collide with other instructions + verify_exit_phi_no_collision(func, exit_block); +} + +/// Phase 132-P2: Verify exit PHI dst ValueIds don't collide with other instructions +/// +/// # Problem +/// +/// If exit_phi_builder uses builder.value_gen.next() (module-level) instead of +/// func.next_value_id() (function-level), it can allocate ValueIds that collide +/// with existing instructions in the function. +/// +/// Example collision: +/// - bb0: %1 = const 0 (counter init) +/// - bb3: %1 = phi ... (exit PHI - collision!) +/// +/// This causes LLVM backend errors: +/// "Cannot overwrite PHI dst=1. ValueId namespace collision detected." +/// +/// # Contract +/// +/// All exit PHI dst ValueIds must be unique within the function and not +/// overwrite any existing instruction dst. +/// +/// # Panics +/// +/// Panics if any exit PHI dst collides with an existing instruction dst. +#[cfg(debug_assertions)] +fn verify_exit_phi_no_collision(func: &MirFunction, exit_block: BasicBlockId) { + let exit_block_data = match func.blocks.get(&exit_block) { + Some(block) => block, + None => return, // Block not found, other verification will catch this + }; + + // Collect all exit PHI dsts + let mut exit_phi_dsts = std::collections::HashSet::new(); + for instr in &exit_block_data.instructions { + if let MirInstruction::Phi { dst, .. } = instr { + exit_phi_dsts.insert(*dst); + } + } + + if exit_phi_dsts.is_empty() { + return; // No exit PHIs, nothing to verify + } + + // Collect all instruction dsts in the entire function (excluding PHIs) + let mut all_non_phi_dsts = std::collections::HashSet::new(); + for (block_id, block) in &func.blocks { + if *block_id == exit_block { + // For exit block, only check non-PHI instructions + for instr in &block.instructions { + if !matches!(instr, MirInstruction::Phi { .. }) { + if let Some(dst) = get_instruction_dst(instr) { + all_non_phi_dsts.insert(dst); + } + } + } + } else { + // For other blocks, check all instructions + for instr in &block.instructions { + if let Some(dst) = get_instruction_dst(instr) { + all_non_phi_dsts.insert(dst); + } + } + } + } + + // Check for collisions + for phi_dst in &exit_phi_dsts { + if all_non_phi_dsts.contains(phi_dst) { + // Find which instruction collides + for (block_id, block) in &func.blocks { + for instr in &block.instructions { + if matches!(instr, MirInstruction::Phi { .. }) && *block_id == exit_block { + continue; // Skip exit PHIs themselves + } + if let Some(dst) = get_instruction_dst(instr) { + if dst == *phi_dst { + panic!( + "[JoinIRVerifier/Phase132-P2] Exit PHI dst {:?} collides with instruction in block {}: {:?}\n\ + This indicates exit_phi_builder used module-level value_gen.next() instead of function-level next_value_id().\n\ + Fix: Use func.next_value_id() in exit_phi_builder.rs", + phi_dst, block_id.0, instr + ); + } + } + } + } + } + } +} + +/// Helper: Extract dst ValueId from MirInstruction +#[cfg(debug_assertions)] +fn get_instruction_dst(instr: &MirInstruction) -> Option { + use MirInstruction; + match instr { + MirInstruction::Const { dst, .. } + | MirInstruction::Load { dst, .. } + | MirInstruction::UnaryOp { dst, .. } + | MirInstruction::BinOp { dst, .. } + | MirInstruction::Compare { dst, .. } + | MirInstruction::TypeOp { dst, .. } + | MirInstruction::NewBox { dst, .. } + | MirInstruction::NewClosure { dst, .. } + | MirInstruction::Copy { dst, .. } + | MirInstruction::Cast { dst, .. } + | MirInstruction::TypeCheck { dst, .. } + | MirInstruction::Phi { dst, .. } + | MirInstruction::ArrayGet { dst, .. } + | MirInstruction::RefNew { dst, .. } + | MirInstruction::RefGet { dst, .. } + | MirInstruction::WeakNew { dst, .. } + | MirInstruction::WeakLoad { dst, .. } + | MirInstruction::WeakRef { dst, .. } + | MirInstruction::FutureNew { dst, .. } + | MirInstruction::Await { dst, .. } => Some(*dst), + MirInstruction::BoxCall { dst, .. } + | MirInstruction::ExternCall { dst, .. } + | MirInstruction::Call { dst, .. } + | MirInstruction::PluginInvoke { dst, .. } => *dst, + _ => None, + } } #[cfg(debug_assertions)]