diff --git a/docs/development/current/main/phase177-carrier-evolution.md b/docs/development/current/main/phase177-carrier-evolution.md new file mode 100644 index 00000000..77a49567 --- /dev/null +++ b/docs/development/current/main/phase177-carrier-evolution.md @@ -0,0 +1,165 @@ +# Phase 177: Carrier Evolution - min から Production へ + +## 視覚的比較: ループ構造の進化 + +### Phase 174: min（1-carrier） +``` +Input: "hello world"" + ^start ^target + +loop(pos < len) { + ch = s[pos] + if ch == '"' → break ✓ + else → pos++ +} + +Carriers: 1 +- pos (ループ変数) + +Pattern: 4 (Loop + If PHI) +Exit: pos = 11 +``` + +### Phase 175/176: min2（2-carrier） +``` +Input: "hello world"" + ^start ^target + +loop(pos < len) { + ch = s[pos] + if ch == '"' → break ✓ + else → result += ch + pos++ +} + +Carriers: 2 +- pos (ループ変数) +- result (バッファ) + +Pattern: 4 (Loop + If PHI) +Exit: pos = 11, result = "hello world" +``` + +### Phase 177-A: Simple Case（2-carrier, Production-like） +``` +Input: "hello world"" + ^start ^target + +loop(p < len) { + ch = s[p] + if ch == '"' → break ✓ + else → str += ch + p++ +} + +Carriers: 2 +- p (ループ変数) +- str (バッファ) + +Pattern: 4 (Loop + If PHI) +Exit: p = 11, str = "hello world" +``` + +### Phase 178: Escape Handling（2-carrier + continue） +``` +Input: "hello \"world\""" + ^start ^escape ^target + +loop(p < len) { + ch = s[p] + if ch == '"' → break ✓ + if ch == '\\' → str += ch + p++ + str += s[p] + p++ + continue ←← 新要素 + else → str += ch + p++ +} + +Carriers: 2 (変わらず) +- p (ループ変数) +- str (バッファ) + +Pattern: 4? (continue 対応は未検証) +Exit: p = 15, str = "hello \"world\"" +``` + +### Phase 179: Full Production（2-carrier + early return） +``` +Input: "hello \"world\""" + ^start ^escape ^target + +loop(p < len) { + ch = s[p] + if ch == '"' → return MapBox {...} ✓ ←← early return + if ch == '\\' → if p+1 >= len → return null ←← error + str += ch + p++ + str += s[p] + p++ + continue + else → str += ch + p++ +} +return null ←← ループ終端エラー + +Carriers: 2 (変わらず) +- p (ループ変数) +- str (バッファ) + +Pattern: 4? (early return 対応は未検証) +Exit: 正常: MapBox, 異常: null +``` + +## Carrier 安定性分析 + +### 重要な発見 +**Phase 174 → 179 を通じて Carrier 数は安定（1 → 2）** + +| Phase | Carriers | 新要素 | P5昇格候補 | +|-------|----------|--------|-----------| +| 174 | 1 (pos) | - | なし | +| 175/176 | 2 (pos + result) | バッファ追加 | なし | +| 177-A | 2 (p + str) | - | なし | +| 178 | 2 (p + str) | continue | なし（確認中） | +| 179 | 2 (p + str) | early return | なし（確認中） | + +### P5昇格候補の不在 +**Trim と異なり、`is_ch_match` 相当は不要** + +理由: +- Trim: `is_ch_match` が「次も空白か？」を決定（**次の判断に影響**） +- _parse_string: `ch == '"'` は「今終了か？」のみ（**次に影響しない**） + +``` +Trim の制御フロー: + is_ch_match = (ch == ' ') + if is_ch_match → pos++ → 次も is_ch_match を評価 ←← 連鎖 + +_parse_string の制御フロー: + if ch == '"' → break → 終了 + else → str += ch, p++ → 次は独立判断 ←← 連鎖なし +``` + +## JoinIR Pattern 対応予測 + +| Phase | Pattern 候補 | 理由 | +|-------|-------------|------| +| 177-A | Pattern4 | Loop + If PHI + break（実装済み） | +| 178 | Pattern4? | continue は Pattern4-with-continue（要実装確認） | +| 179 | Pattern5? | early return は新パターン候補（要設計） | + +## まとめ + +### 段階的検証戦略 +1. **Phase 177-A**: min2 と同型 → P5 安定性確認 +2. **Phase 178**: continue 追加 → JoinIR 拡張必要性評価 +3. **Phase 179**: early return 追加 → Pattern5 設計判断 + +### Carrier 設計の教訓 +- **最小構成で開始**: 1-carrier (Phase 174) +- **段階的拡張**: 2-carrier (Phase 175/176) +- **Production 適用**: 構造は変えず、制御フローのみ追加（Phase 177+） + +→ **「Carrier 数を固定して制御フローを段階的に複雑化」が正解** diff --git a/docs/development/current/main/phase177-parse-string-design.md b/docs/development/current/main/phase177-parse-string-design.md new file mode 100644 index 00000000..ae7a3ce3 --- /dev/null +++ b/docs/development/current/main/phase177-parse-string-design.md @@ -0,0 +1,178 @@ +# Phase 177: _parse_string 本体 JoinIR 適用設計 + +## 目的 +Production `JsonParserBox._parse_string` メソッドに JoinIR P5 パイプラインを適用する。 +Phase 174/175/176 で確立した「1-carrier → 2-carrier」検証の成果を、実際の JSON パーサーに展開する。 + +## 本番ループ構造（lines 144-181） + +### 前提条件 +- 開始位置 `pos` は `"` を指している（line 145 でチェック） +- ループ開始時 `p = pos + 1`（引用符の次の文字から開始） + +### ループ本体（lines 150-178） +```hako +loop(p < s.length()) { + local ch = s.substring(p, p+1) + + if ch == '"' { + // End of string (lines 153-160) + local result = new MapBox() + result.set("value", me._unescape_string(str)) + result.set("pos", p + 1) + result.set("type", "string") + return result // ← Early return (通常の exit) + } + + if ch == "\\" { + // Escape sequence (lines 162-174) + local has_next = 0 + if p + 1 < s.length() { has_next = 1 } + + if has_next == 0 { return null } // ← Early return (エラー) + + str = str + ch + p = p + 1 + str = str + s.substring(p, p+1) + p = p + 1 + continue // ← ループ継続 + } + + str = str + ch // 通常文字の追加 + p = p + 1 +} + +return null // ← ループ終端に到達（エラー） +``` + +### 制御フロー分岐 +1. **終端クォート検出** (`ch == '"'`): 成功 return +2. **エスケープ検出** (`ch == "\\"`) + continue: 2文字消費してループ継続 +3. **通常文字**: バッファ蓄積 + 位置進行 +4. **ループ終端**: 引用符が閉じていない（エラー） + +## Carriers 分析 + +| 変数 | 初期値 | 役割 | 更新式 | P5 昇格対象？ | 備考 | +|------|--------|------|--------|---------------|------| +| `p` | `pos + 1` | カウンタ（位置） | `p = p + 1` | **N（ループ変数）** | 条件式に使用 | +| `str` | `""` | バッファ（蓄積） | `str = str + ch` | **N（通常キャリア）** | エスケープ時2回更新 | +| `has_next` | - | 一時変数（フラグ） | - | - | ループ内のみ有効 | +| `is_escape` | - | （潜在的フラグ） | - | - | 明示的変数なし | + +### 重要な発見 +- **エスケープ処理は continue 経由**: `p` と `str` を 2 回更新してから continue +- **Early return が 2 箇所**: 成功 return (line 159) とエラー return (line 167) +- **通常キャリアのみ**: P5 昇格対象（`is_ch_match` 相当）は**不要** + +## min ケースとの差分 + +### 共通点 +| 項目 | min (Phase 174) | min2 (Phase 175) | 本番 (Phase 177) | +|------|-----------------|------------------|------------------| +| ループ変数 | `pos` | `pos` | `p` | +| バッファ | なし | `result` | `str` | +| 終端条件 | `ch == '"'` → break | `ch == '"'` → break | `ch == '"'` → return | +| 通常処理 | `pos++` | `result += ch; pos++` | `str += ch; p++` | + +### 差分 +| 項目 | min/min2 | 本番 | +|------|----------|------| +| 終端処理 | `break` のみ | Early `return` (MapBox 返却) | +| エスケープ処理 | なし | `continue` を使った複雑な分岐 | +| エラー処理 | なし | ループ内・外に `return null` | + +## 方針決定: 段階的アプローチ + +### Phase 177-A: Simple Case（今回） +**対象**: エスケープ**なし**・終端クォート検出のみ +- **ループ構造**: min2 と完全同型（`p` + `str` の 2-carrier） +- **終端処理**: `break` → 直後に MapBox 構築（return 代替） +- **目的**: P5 パイプラインが「2-carrier + break」で動作することを確認 + +```hako +// Simplified for Phase 177-A +loop(p < s.length()) { + local ch = s.substring(p, p+1) + if ch == '"' { + break // P5: Pattern4 対応（Loop+If PHI merge） + } else { + str = str + ch + p = p + 1 + } +} +// break 後: MapBox 構築 +``` + +### Phase 178: Escape Handling（次回） +**追加要素**: +- `continue` 分岐（エスケープ処理） +- Early return（エラーハンドリング） +- P5 昇格キャリア候補の検討（`is_escape` フラグ？） + +### Phase 179: Full Production（最終） +**統合**: +- `_unescape_string()` 呼び出し +- 完全なエラーハンドリング +- 本番同等の MapBox 返却 + +## Phase 177-A 実装計画 + +### Test Case: `test_jsonparser_parse_string_simple.hako` +```hako +// Phase 177-A: Production-like simple case +static box JsonParserStringTest3 { + parse_string_simple() { + local s = "hello world\"" + local p = 0 // pos + 1 相当（簡略化のため 0 から開始） + local str = "" + local len = s.length() + + // 2-carrier loop (min2 と同型) + loop(p < len) { + local ch = s.substring(p, p+1) + if ch == "\"" { + break + } else { + str = str + ch + p = p + 1 + } + } + + // Post-loop: 結果出力（MapBox 構築の代替） + print("Parsed string: ") + print(str) + print(", final pos: ") + print(p) + } + + main() { + me.parse_string_simple() + return "OK" + } +} +``` + +### 期待される MIR 構造 +- **Pattern4 検出**: Loop + If PHI merge（Phase 170 実装済み） +- **2 carriers**: `p` (ループ変数) + `str` (バッファ) +- **Exit PHI**: ループ後の `p` と `str` が正しく伝播 + +### 検証項目 +1. ✅ P5 パイプライン通過（JoinIR → MIR） +2. ✅ 2-carrier の正しい伝播（`p` と `str`） +3. ✅ `break` 後の変数値が正しく使用可能 + +## 成功基準 +- [ ] `test_jsonparser_parse_string_simple.hako` が実行成功 +- [ ] MIR ダンプで Pattern4 検出確認 +- [ ] 出力: `Parsed string: hello world, final pos: 11` + +## 次のステップ（Phase 178） +- `continue` 分岐の追加（エスケープ処理） +- P5 昇格キャリア候補の検討（必要性を再評価） +- Early return 対応（JoinIR での処理検討） + +## まとめ +**Phase 177-A では、min2 と同型の Simple Case を Production 環境に適用する。** +エスケープ処理は Phase 178 以降に回し、まず「2-carrier + break」の動作確認を優先する。 diff --git a/src/mir/builder/control_flow/joinir/merge/block_allocator.rs b/src/mir/builder/control_flow/joinir/merge/block_allocator.rs index ab8a86e6..1226bd03 100644 --- a/src/mir/builder/control_flow/joinir/merge/block_allocator.rs +++ b/src/mir/builder/control_flow/joinir/merge/block_allocator.rs @@ -12,15 +12,20 @@ use super::super::trace; /// Phase 1: Allocate new block IDs for ALL functions (Phase 189) /// /// DETERMINISM: Sort functions and blocks by name/ID to ensure consistent iteration order +/// +/// Returns: (JoinIrIdRemapper, exit_block_id) pub(super) fn allocate_blocks( builder: &mut crate::mir::builder::MirBuilder, mir_module: &MirModule, debug: bool, -) -> Result { +) -> Result<(JoinIrIdRemapper, crate::mir::BasicBlockId), String> { let mut remapper = JoinIrIdRemapper::new(); - // Create exit block for Return conversion (single for all functions) - let _exit_block_id = builder.block_gen.next(); + // Phase 177-3: Allocate exit block FIRST to ensure it doesn't conflict with JoinIR blocks + // This exit_block_id will be returned and used by instruction_rewriter and exit_phi_builder + let exit_block_id = builder.block_gen.next(); + + eprintln!("[cf_loop/joinir/block_allocator] Phase 177-3: Allocated exit_block_id = {:?}", exit_block_id); // Phase 195: Use unified trace trace::trace().blocks("allocator", "Phase 189: Allocating block IDs for all functions"); @@ -61,5 +66,6 @@ pub(super) fn allocate_blocks( ); } - Ok(remapper) + // Phase 177-3: Return both remapper and exit_block_id + Ok((remapper, exit_block_id)) } 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 f3e4c789..71880c8b 100644 --- a/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs +++ b/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs @@ -30,6 +30,11 @@ use std::collections::HashMap; /// Uses extracted modules: /// - tail_call_classifier: TailCallKind classification /// - merge_result: MergeResult data structure +/// +/// # Phase 177-3 +/// +/// `exit_block_id` is now passed in from block_allocator to ensure it doesn't +/// conflict with JoinIR function blocks. pub(super) fn merge_and_rewrite( builder: &mut crate::mir::builder::MirBuilder, mir_module: &MirModule, @@ -38,13 +43,11 @@ pub(super) fn merge_and_rewrite( function_params: &HashMap>, boundary: Option<&JoinInlineBoundary>, loop_header_phi_info: &mut LoopHeaderPhiInfo, + exit_block_id: BasicBlockId, debug: bool, -) -> Result { - // Create exit block for Return conversion (single for all functions) - let exit_block_id = builder.block_gen.next(); - if debug { - eprintln!("[cf_loop/joinir] Exit block: {:?}", exit_block_id); - } +)-> Result { + // Phase 177-3: exit_block_id is now passed in from block_allocator + eprintln!("[cf_loop/joinir/instruction_rewriter] Phase 177-3: Using exit_block_id = {:?}", exit_block_id); // Phase 189 FIX: Build set of boundary join_inputs to skip their Const initializers let boundary_input_set: std::collections::HashSet = boundary @@ -172,8 +175,58 @@ pub(super) fn merge_and_rewrite( let mut found_tail_call = false; let mut tail_call_target: Option<(BasicBlockId, Vec)> = None; + // Phase 177-3: Check if this block is the loop header with PHI nodes + let is_loop_header_with_phi = is_loop_entry_point && !loop_header_phi_info.carrier_phis.is_empty(); + + if is_loop_entry_point { + eprintln!( + "[cf_loop/joinir] Phase 177-3 DEBUG: is_loop_entry_point={}, carrier_phis.len()={}, is_loop_header_with_phi={}", + is_loop_entry_point, + loop_header_phi_info.carrier_phis.len(), + is_loop_header_with_phi + ); + } + + // Phase 177-3: Collect PHI dst IDs that will be created for this block + // (if this is the loop header) + let phi_dst_ids_for_block: std::collections::HashSet = if is_loop_header_with_phi { + loop_header_phi_info.carrier_phis.values() + .map(|entry| entry.phi_dst) + .collect() + } else { + std::collections::HashSet::new() + }; + + if is_loop_header_with_phi && !phi_dst_ids_for_block.is_empty() { + eprintln!( + "[cf_loop/joinir] Phase 177-3: Loop header with {} PHI dsts to protect: {:?}", + phi_dst_ids_for_block.len(), + phi_dst_ids_for_block + ); + } + // First pass: Process all instructions, identify tail calls for inst in &old_block.instructions { + // Phase 177-3: Skip Copy instructions that would overwrite PHI dsts + // The loop header PHIs will provide these values, so copies are redundant/harmful + if is_loop_header_with_phi { + if let MirInstruction::Copy { dst, src } = inst { + // Check if this copy's dst is a PHI dst (after remapping) + let dst_remapped = remapper.get_value(*dst).unwrap_or(*dst); + eprintln!( + "[cf_loop/joinir] Phase 177-3 DEBUG: Copy {:?} = {:?}, dst_remapped = {:?}, in phi_dsts = {}", + dst, src, dst_remapped, phi_dst_ids_for_block.contains(&dst_remapped) + ); + if phi_dst_ids_for_block.contains(&dst_remapped) { + eprintln!( + "[cf_loop/joinir] Phase 177-3: ✅ Skipping loop header Copy to PHI dst {:?} (original {:?})", + dst_remapped, dst + ); + continue; // Skip - would overwrite PHI + } + } + } + // Phase 189: Skip Const String instructions that define function names if let MirInstruction::Const { dst, value } = inst { if let crate::mir::types::ConstValue::String(_) = value { @@ -653,15 +706,30 @@ pub(super) fn merge_and_rewrite( } } + // Phase 177-3: Collect PHI dst IDs from loop_header_phi_info + let phi_dst_ids: std::collections::HashSet = loop_header_phi_info + .carrier_phis + .values() + .map(|entry| entry.phi_dst) + .collect(); + // Use BoundaryInjector to inject Copy instructions + // Phase 177-3 Option B: Returns reallocations map for condition_bindings with PHI collisions if let Some(ref mut current_func) = builder.current_function { - BoundaryInjector::inject_boundary_copies( + let _reallocations = BoundaryInjector::inject_boundary_copies( current_func, entry_block_remapped, boundary, &value_map_for_injector, + &phi_dst_ids, debug, )?; + + // Note: reallocations map is returned but not currently used. + // If condition variables need to be referenced after this point, + // the reallocations map would need to be applied to update references. + // For now, condition_bindings are read-only variables used only in + // the loop condition, so no further updates are needed. } } diff --git a/src/mir/builder/control_flow/joinir/merge/mod.rs b/src/mir/builder/control_flow/joinir/merge/mod.rs index 6e0c81f2..8590558f 100644 --- a/src/mir/builder/control_flow/joinir/merge/mod.rs +++ b/src/mir/builder/control_flow/joinir/merge/mod.rs @@ -80,7 +80,8 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( } // Phase 1: Allocate block IDs for all functions - let mut remapper = block_allocator::allocate_blocks(builder, mir_module, debug)?; + // Phase 177-3: block_allocator now returns exit_block_id to avoid conflicts + let (mut remapper, exit_block_id) = block_allocator::allocate_blocks(builder, mir_module, debug)?; // Phase 2: Collect values from all functions let (mut used_values, value_to_func_name, function_params) = @@ -113,6 +114,30 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( // Phase 3: Remap ValueIds remap_values(builder, &used_values, &mut remapper, debug)?; + // Phase 177-3 DEBUG: Verify remapper state after Phase 3 + eprintln!("[DEBUG-177] === Remapper state after Phase 3 ==="); + eprintln!("[DEBUG-177] used_values count: {}", used_values.len()); + for value_id in &used_values { + if let Some(remapped) = remapper.get_value(*value_id) { + eprintln!("[DEBUG-177] JoinIR {:?} → Host {:?}", value_id, remapped); + } else { + eprintln!("[DEBUG-177] JoinIR {:?} → NOT FOUND ❌", value_id); + } + } + + // Check condition_bindings specifically + if let Some(boundary) = boundary { + eprintln!("[DEBUG-177] === Condition bindings check ==="); + for binding in &boundary.condition_bindings { + let lookup_result = remapper.get_value(binding.join_value); + eprintln!( + "[DEBUG-177] '{}': JoinIR {:?} → {:?}", + binding.name, binding.join_value, lookup_result + ); + } + } + eprintln!("[DEBUG-177] =============================="); + // Phase 3.5: Build loop header PHIs (if loop pattern with loop_var_name) // // We need to know PHI dsts before instruction_rewriter runs, so that: @@ -215,9 +240,47 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( // 2. loop body uses PHI result // 3. tail call args are correctly routed - // Map main's parameters - // MIR function keys use join_func_N format from join_func_name() + // Phase 177-3 fix: Protect condition-ONLY bindings from being overridden to PHI dsts + // + // Problem: condition_bindings may contain: + // 1. True condition-only variables (e.g., 'limit' in loop(i < limit)) - NOT carriers + // 2. Body-only carriers added by Phase 176-5 (e.g., 'result') - ARE carriers + // + // We must ONLY protect (1), not (2), because: + // - Condition-only vars should keep their HOST mapping (e.g., limit = %8) + // - Body-only carriers MUST be remapped to PHI dsts (e.g., result = %24) + // + // Solution: Protect condition_bindings that are NOT in exit_bindings (i.e., not carriers) + let carrier_names: std::collections::HashSet<&str> = boundary + .exit_bindings + .iter() + .map(|eb| eb.carrier_name.as_str()) + .collect(); + + let condition_binding_ids: std::collections::HashSet = boundary + .condition_bindings + .iter() + .filter(|cb| !carrier_names.contains(cb.name.as_str())) + .map(|cb| cb.join_value) + .collect(); + + if !condition_binding_ids.is_empty() { + eprintln!( + "[cf_loop/joinir] Phase 177-3: Protected ValueIds (condition-only, not carriers): {:?}", + condition_binding_ids + ); + for cb in &boundary.condition_bindings { + let is_carrier = carrier_names.contains(cb.name.as_str()); + eprintln!( + "[cf_loop/joinir] Phase 177-3: '{}': JoinIR {:?} (carrier={})", + cb.name, cb.join_value, is_carrier + ); + } + } + let main_func_name = "join_func_0"; + let loop_step_func_name = "join_func_1"; + if function_params.get(main_func_name).is_none() { eprintln!( "[cf_loop/joinir] WARNING: function_params.get('{}') returned None. Available keys: {:?}", @@ -226,30 +289,57 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( ); } if let Some(main_params) = function_params.get(main_func_name) { - if debug { - eprintln!( - "[cf_loop/joinir] Phase 33-21: main ({}) params: {:?}", - main_func_name, main_params - ); - } + eprintln!( + "[DEBUG-177] Phase 33-21: main ({}) params: {:?}", + main_func_name, main_params + ); + eprintln!( + "[DEBUG-177] Phase 33-21: carrier_phis count: {}, names: {:?}", + phi_info.carrier_phis.len(), + phi_info.carrier_phis.iter().map(|(n, _)| n.as_str()).collect::>() + ); // Map main's parameters to header PHI dsts // main params: [i_init, carrier1_init, ...] // carrier_phis: [("i", entry), ("sum", entry), ...] for (idx, (carrier_name, entry)) in phi_info.carrier_phis.iter().enumerate() { if let Some(&main_param) = main_params.get(idx) { - if debug { + // Phase 177-3: Don't override condition_bindings + if condition_binding_ids.contains(&main_param) { eprintln!( - "[cf_loop/joinir] Phase 33-21: REMAP main param {:?} → {:?} ('{}')", - main_param, entry.phi_dst, carrier_name + "[cf_loop/joinir] Phase 177-3: Skipping override for condition_binding {:?} ('{}')", + main_param, carrier_name ); + continue; } + eprintln!( + "[DEBUG-177] Phase 33-21: REMAP main param[{}] {:?} → {:?} ('{}')", + idx, main_param, entry.phi_dst, carrier_name + ); remapper.set_value(main_param, entry.phi_dst); } } } + // Phase 177-3-B: Handle body-only carriers + // These are carriers in carrier_phis that are NOT in main function params. + // They appear in condition_bindings (added by Phase 176-5) but need PHI remapping. + for (carrier_name, entry) in &phi_info.carrier_phis { + // Check if this carrier has a condition_binding + if let Some(binding) = boundary.condition_bindings.iter().find(|cb| cb.name == *carrier_name) { + // Skip if it's a true condition-only variable (already protected above) + if condition_binding_ids.contains(&binding.join_value) { + continue; + } + // This is a body-only carrier - remap it to PHI dst + eprintln!( + "[cf_loop/joinir] Phase 177-3-B: Body-only carrier '{}': JoinIR {:?} → PHI {:?}", + carrier_name, binding.join_value, entry.phi_dst + ); + remapper.set_value(binding.join_value, entry.phi_dst); + } + } + // Map loop_step's parameters - let loop_step_func_name = "join_func_1"; if debug { eprintln!( "[cf_loop/joinir] Phase 33-21: function_params keys: {:?}", @@ -275,6 +365,14 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( // carrier_phis: [("i", entry), ("sum", entry), ...] for (idx, (carrier_name, entry)) in phi_info.carrier_phis.iter().enumerate() { if let Some(&loop_step_param) = loop_step_params.get(idx) { + // Phase 177-3: Don't override condition_bindings + if condition_binding_ids.contains(&loop_step_param) { + eprintln!( + "[cf_loop/joinir] Phase 177-3: Skipping override for condition_binding {:?} ('{}')", + loop_step_param, carrier_name + ); + continue; + } if debug { eprintln!( "[cf_loop/joinir] Phase 33-21: REMAP loop_step param {:?} → {:?} ('{}')", @@ -290,11 +388,18 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( // Fallback: Use old behavior (ValueId(0), ValueId(1), ...) // This handles patterns that don't have loop_step function if let Some(phi_dst) = phi_info.get_carrier_phi(loop_var_name) { - remapper.set_value(ValueId(0), phi_dst); - if debug { + // Phase 177-3: Don't override condition_bindings + if !condition_binding_ids.contains(&ValueId(0)) { + remapper.set_value(ValueId(0), phi_dst); + if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-16 fallback: Override remap ValueId(0) → {:?} (PHI dst)", + phi_dst + ); + } + } else { eprintln!( - "[cf_loop/joinir] Phase 33-16 fallback: Override remap ValueId(0) → {:?} (PHI dst)", - phi_dst + "[cf_loop/joinir] Phase 177-3 fallback: Skipping override for condition_binding ValueId(0)" ); } } @@ -303,16 +408,34 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( continue; } let join_value_id = ValueId(idx as u32); - remapper.set_value(join_value_id, entry.phi_dst); - if debug { + // Phase 177-3: Don't override condition_bindings + if !condition_binding_ids.contains(&join_value_id) { + remapper.set_value(join_value_id, entry.phi_dst); + if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-20 fallback: Override remap {:?} → {:?} (carrier '{}' PHI dst)", + join_value_id, entry.phi_dst, carrier_name + ); + } + } else { eprintln!( - "[cf_loop/joinir] Phase 33-20 fallback: Override remap {:?} → {:?} (carrier '{}' PHI dst)", - join_value_id, entry.phi_dst, carrier_name + "[cf_loop/joinir] Phase 177-3 fallback: Skipping override for condition_binding {:?} ('{}')", + join_value_id, carrier_name ); } } } + // Phase 177-3 DEBUG: Check remapper after Phase 33-21 overrides + eprintln!("[DEBUG-177] === Remapper state after Phase 33-21 ==="); + for binding in &boundary.condition_bindings { + let lookup_result = remapper.get_value(binding.join_value); + eprintln!( + "[DEBUG-177] '{}': JoinIR {:?} → {:?} (after 33-21)", + binding.name, binding.join_value, lookup_result + ); + } + phi_info } else { LoopHeaderPhiInfo::empty(entry_block_remapped) @@ -323,6 +446,7 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( // Phase 4: Merge blocks and rewrite instructions // Phase 33-16: Pass mutable loop_header_phi_info for latch_incoming tracking + // Phase 177-3: Pass exit_block_id from allocator to avoid conflicts let merge_result = instruction_rewriter::merge_and_rewrite( builder, mir_module, @@ -331,6 +455,7 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( &function_params, boundary, &mut loop_header_phi_info, + exit_block_id, debug, )?; diff --git a/src/mir/builder/joinir_inline_boundary_injector.rs b/src/mir/builder/joinir_inline_boundary_injector.rs index 86bdfdd6..308f718b 100644 --- a/src/mir/builder/joinir_inline_boundary_injector.rs +++ b/src/mir/builder/joinir_inline_boundary_injector.rs @@ -39,13 +39,26 @@ impl BoundaryInjector { /// When `boundary.loop_var_name` is set, ALL carriers (loop var + other carriers /// from exit_bindings) are handled by header PHIs. We skip ALL join_inputs /// Copy instructions to avoid overwriting the PHI results. + /// + /// # Phase 177-3: PHI Collision Avoidance (Option B) + /// + /// When `phi_dst_ids` contains ValueIds of existing PHI dsts in the header block, + /// we allocate a NEW ValueId for condition_bindings instead of skipping the copy. + /// This ensures the condition variable is available even when its remapped ValueId + /// collides with a PHI dst. + /// + /// # Returns + /// + /// Returns a HashMap mapping original ValueIds to their reallocated ValueIds + /// (only for condition_bindings that had collisions). pub fn inject_boundary_copies( func: &mut MirFunction, entry_block_id: BasicBlockId, boundary: &JoinInlineBoundary, value_map: &HashMap, + phi_dst_ids: &std::collections::HashSet, debug: bool, - ) -> Result<(), String> { + ) -> Result, String> { // Phase 33-20: When loop_var_name is set, ALL join_inputs are handled by header PHIs // This includes the loop variable AND all other carriers from exit_bindings. // We skip ALL join_inputs Copy instructions, only condition_bindings remain. @@ -59,7 +72,8 @@ impl BoundaryInjector { }; let total_inputs = effective_join_inputs + boundary.condition_bindings.len(); if total_inputs == 0 { - return Ok(()); + // No inputs to process, return empty reallocations map + return Ok(HashMap::new()); } if debug { @@ -73,7 +87,38 @@ impl BoundaryInjector { ); } - // Entry block を取得 + // Phase 177-3: Use PHI dst IDs passed from caller + if debug && !phi_dst_ids.is_empty() { + eprintln!( + "[BoundaryInjector] Phase 177-3: Received {} PHI dst IDs to avoid: {:?}", + phi_dst_ids.len(), + phi_dst_ids + ); + } + + // Phase 177-3 Option B: First pass - allocate all new ValueIds for PHI collisions + // We need to do this BEFORE acquiring the entry_block reference to avoid borrow conflicts + let mut reallocations = HashMap::new(); + + for binding in &boundary.condition_bindings { + let remapped_join = value_map.get(&binding.join_value).copied().unwrap_or(binding.join_value); + + if phi_dst_ids.contains(&remapped_join) { + // Collision detected! Allocate a fresh ValueId + let fresh_dst = func.next_value_id(); + + if debug { + eprintln!( + "[BoundaryInjector] Phase 177-3 Option B: PHI collision for condition binding '{}': {:?} → reallocated to {:?}", + binding.name, remapped_join, fresh_dst + ); + } + + reallocations.insert(binding.join_value, fresh_dst); + } + } + + // Now get entry block reference (after all ValueId allocations are done) let entry_block = func .get_block_mut(entry_block_id) .ok_or(format!("Entry block {:?} not found", entry_block_id))?; @@ -110,17 +155,34 @@ impl BoundaryInjector { } } + // Phase 177-3 DEBUG: Check value_map in BoundaryInjector + if debug { + eprintln!("[DEBUG-177] === BoundaryInjector value_map ==="); + for binding in &boundary.condition_bindings { + let lookup = value_map.get(&binding.join_value); + eprintln!( + "[DEBUG-177] '{}': JoinIR {:?} → {:?}", + binding.name, binding.join_value, lookup + ); + } + } + // Phase 171-fix: Inject Copy instructions for condition_bindings (condition-only variables) // These variables are read-only and used ONLY in the loop condition. // Each binding explicitly specifies HOST ValueId → JoinIR ValueId mapping. // We inject Copy: remapped_join_value = Copy host_value + // + // Phase 177-3 Option B: Use pre-allocated reallocations for PHI collision cases for binding in &boundary.condition_bindings { // Look up the remapped JoinIR ValueId from value_map let remapped_join = value_map.get(&binding.join_value).copied().unwrap_or(binding.join_value); - // Copy instruction: remapped_join_value = Copy host_value + // Phase 177-3 Option B: Check if this binding was reallocated (PHI collision case) + let final_dst = reallocations.get(&binding.join_value).copied().unwrap_or(remapped_join); + + // Copy instruction: final_dst = Copy host_value let copy_inst = MirInstruction::Copy { - dst: remapped_join, + dst: final_dst, src: binding.host_value, }; @@ -128,8 +190,9 @@ impl BoundaryInjector { if debug { eprintln!( - "[BoundaryInjector] Condition binding '{}': Copy {:?} = Copy {:?} (JoinIR {:?} → remapped {:?})", - binding.name, remapped_join, binding.host_value, binding.join_value, remapped_join + "[BoundaryInjector] Condition binding '{}': Copy {:?} = Copy {:?} (JoinIR {:?} → remapped {:?}{})", + binding.name, final_dst, binding.host_value, binding.join_value, remapped_join, + if final_dst != remapped_join { format!(" → reallocated {:?}", final_dst) } else { String::new() } ); } } @@ -145,7 +208,8 @@ impl BoundaryInjector { entry_block.instruction_spans.insert(0, default_span); } - Ok(()) + // Return reallocations map for condition_bindings that had PHI collisions + Ok(reallocations) } }