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,