From 1510dcb7d8c6da2e4fdd63de036eced84f6d2827 Mon Sep 17 00:00:00 2001 From: nyash-codex Date: Sun, 14 Dec 2025 06:52:50 +0900 Subject: [PATCH] =?UTF-8?q?fix(llvm):=20Phase=20131-6=20=E8=AA=BF=E6=9F=BB?= =?UTF-8?q?=20-=20TAG-RUN=203=E3=83=90=E3=82=B0=E7=99=BA=E8=A6=8B=EF=BC=88?= =?UTF-8?q?1=E4=BF=AE=E6=AD=A3/1=E9=83=A8=E5=88=86/1=E6=9C=AA=E4=BF=AE?= =?UTF-8?q?=E6=AD=A3=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 131-6: Infinite Loop Bug 調査完了 発見したバグ（3件）: 1. Bug #1: Copy-to-PHI 命令（SSA 違反）✅ 修正完了 - instruction_rewriter.rs: PHI destination への Copy をスキップ 2. Bug #2: Type Inference 混同（String vs Integer）⚠️ 部分修正 - binop.py: force_string ロジック削除 3. Bug #3: SSA Dominance Violation ❌ 未修正 - MIR builder が定義前に値を使用（根本問題） 変更ファイル: - src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs: - Lines 428-443: header PHI への Copy スキップ追加 - src/llvm_py/instructions/binop.py: - Lines 128-159: force_string 削除、Phase 131-6 コメント追加 - docs/development/current/main/phase131-3-llvm-lowering-inventory.md: - 3バグの詳細追記 テスト結果: - Case A/B2: ✅ 退行なし - Case B: ❌ infinite loop 継続（Bug #3 が原因） - Simple Add: ❌ 0 を出力（期待: 1） Next: Phase 131-6 続き - MIR SSA dominance 根治 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../phase131-3-llvm-lowering-inventory.md | 44 ++++++++++++++----- src/llvm_py/instructions/binop.py | 29 ++++++++---- .../joinir/merge/instruction_rewriter.rs | 19 ++++++++ 3 files changed, 74 insertions(+), 18 deletions(-) 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 8e58297a..c1564952 100644 --- a/docs/development/current/main/phase131-3-llvm-lowering-inventory.md +++ b/docs/development/current/main/phase131-3-llvm-lowering-inventory.md @@ -259,22 +259,46 @@ Hint: This loop pattern is not supported. All loops must use JoinIR lowering. --- -### 🔥 Priority 3: Fix TAG-RUN (Loop Infinite Iteration) +### 🔥 Priority 3: Fix TAG-RUN (Loop Infinite Iteration) - IN PROGRESS (Phase 131-6) **Target**: Case B (`loop_min_while.hako`) **Issue**: Loop counter not updating, causes infinite loop printing `0` -**Approach**: -1. Save LLVM IR to file for inspection (`target/aot_objects/loop_min_while.ll`) -2. Trace PHI value through store/load chain -3. Identify why loop variable `i` is not incremented -4. Check if this is a harness bug or MIR generation bug +**Phase 131-6 Investigation Results**: -**Files**: -- Python harness IR generation (save .ll file before assembly) -- MIR JSON inspection (verify correct store/load instructions) +#### Bug #1: MIR Copy-to-PHI (FIXED) +- **Location**: `src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs` lines 419-440 +- **Problem**: Parameter binding was generating `Copy { dst: PHI_dst, src: value }` in loop latch +- **Fix**: Added check to skip Copy when `dst` is a header PHI destination +- **Status**: ✅ Copy instruction removed from block 7 -**Expected**: Case B should RUN ✅ (print 0,1,2 and exit) +#### Bug #2: Type Inference - String vs Integer (PARTIAL FIX) +- **Location**: `src/llvm_py/instructions/binop.py` lines 128-153 +- **Problem**: MIR marks `i + 1` as `dst_type: StringBox` (forward-looking hint from `print(i)` usage) +- **Impact**: Python harness was doing string concatenation instead of integer addition +- **Fix Attempted**: Removed `force_string` logic that trusted `dst_type` hint +- **Status**: ⚠️ Partially fixed but infinite loop persists + +#### Bug #3: Instruction Ordering Violation (DISCOVERED) +- **Location**: MIR builder (instruction scheduling) +- **Problem**: Copy instructions emitted AFTER values are used (violates SSA dominance) +- **Example**: `%6 = %4 + %5` appears before `%5 = copy %3` +- **Impact**: LLVM requires strict SSA form, Rust VM tolerates it +- **Status**: ❌ Not yet addressed + +#### Additional Findings +- Simple test `/tmp/test_simple_add.hako` (`i=0; i=i+1; print(i)`) also fails (prints 0 not 1) +- Issue exists even without loops, suggesting fundamental binop/type problem +- String tagging propagation may be marking integer PHI values as strings + +**Next Steps**: +1. Trace `resolver.is_stringish()` to see if integers are being marked as strings +2. Fix MIR instruction scheduling to respect SSA dominance +3. Consider runtime type checking instead of compile-time inference + +**Files Modified**: +- `src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs` (Phase 131-6 fix) +- `src/llvm_py/instructions/binop.py` (Phase 131-6 partial fix) --- diff --git a/src/llvm_py/instructions/binop.py b/src/llvm_py/instructions/binop.py index 0f7aedda..0186c828 100644 --- a/src/llvm_py/instructions/binop.py +++ b/src/llvm_py/instructions/binop.py @@ -125,13 +125,19 @@ def lower_binop( # pointer present? is_ptr_side = (hasattr(lhs_raw, 'type') and isinstance(lhs_raw.type, ir.PointerType)) or \ (hasattr(rhs_raw, 'type') and isinstance(rhs_raw.type, ir.PointerType)) - # Explicit dst_type hint from MIR JSON? - force_string = False - try: - if isinstance(dst_type, dict) and dst_type.get('kind') == 'handle' and dst_type.get('box_type') == 'StringBox': - force_string = True - except Exception: - pass + # Phase 131-6 FIX: Do NOT use dst_type hint from MIR JSON! + # The dst_type is a forward-looking type hint that may be incorrect + # (e.g., "i + 1" gets StringBox hint because i is later used as string in print(i)) + # We should only do string concat if operands are actually strings. + # + # OLD CODE (REMOVED): + # force_string = False + # try: + # if isinstance(dst_type, dict) and dst_type.get('kind') == 'handle' and dst_type.get('box_type') == 'StringBox': + # force_string = True + # except Exception: + # pass + # tagged string handles?（どちらかが string-ish のとき） any_tagged = False try: @@ -143,7 +149,14 @@ def lower_binop( any_tagged = (lhs in resolver.string_literals) or (rhs in resolver.string_literals) except Exception: pass - is_str = force_string or is_ptr_side or any_tagged + # Phase 131-6: Removed force_string from this check + is_str = is_ptr_side or any_tagged + + # Phase 131-6 DEBUG + if os.environ.get('NYASH_BINOP_DEBUG') == '1': + print(f"[binop +] lhs={lhs} rhs={rhs} dst={dst}") + print(f" is_ptr_side={is_ptr_side} any_tagged={any_tagged} is_str={is_str}") + print(f" dst_type={dst_type}") if is_str: # Helper: convert raw or resolved value to string handle def to_handle(raw, val, tag: str, vid: int): diff --git a/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs b/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs index b84d4c7c..5c2d2787 100644 --- a/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs +++ b/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs @@ -417,12 +417,31 @@ pub(super) fn merge_and_rewrite( } } else { // Insert Copy instructions for parameter binding + // Phase 131-6 FIX: Skip Copy when dst is a header PHI destination + // The PHI node will receive the value as an incoming edge instead for (i, arg_val_remapped) in args.iter().enumerate() { if i < target_params.len() { let param_val_original = target_params[i]; if let Some(param_val_remapped) = remapper.get_value(param_val_original) { + // Phase 131-6: Check if this would overwrite a header PHI dst + let is_header_phi_dst = loop_header_phi_info + .carrier_phis + .values() + .any(|entry| entry.phi_dst == param_val_remapped); + + if is_header_phi_dst { + if debug { + eprintln!( + "[cf_loop/joinir] Phase 131-6: Skip param binding to PHI dst {:?} (PHI receives value via incoming edge)", + param_val_remapped + ); + } + // Skip - PHI will receive this value as incoming edge + continue; + } + new_block.instructions.push(MirInstruction::Copy { dst: param_val_remapped, src: *arg_val_remapped,