fix(llvm): Phase 131-6 調査 - TAG-RUN 3バグ発見(1修正/1部分/1未修正)
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 <noreply@anthropic.com>
This commit is contained in:
@ -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)
|
||||
|
||||
---
|
||||
|
||||
|
||||
@ -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):
|
||||
|
||||
@ -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,
|
||||
|
||||
Reference in New Issue
Block a user