diff --git a/docs/development/current/main/phases/phase-286/README.md b/docs/development/current/main/phases/phase-286/README.md index 0f78d06e..637c3df1 100644 --- a/docs/development/current/main/phases/phase-286/README.md +++ b/docs/development/current/main/phases/phase-286/README.md @@ -1,6 +1,6 @@ # Phase 286: JoinIR Line Absorption（JoinIR→CorePlan/Frag 収束） -Status: Planned (design-first) +Status: In Progress (P0, P1, P3, 286C-2 COMPLETE; P2 pending) ## Goal @@ -93,6 +93,54 @@ Phase 286 では JoinIR line を “第2の lowerer” として放置せず、* - 代表 1 パターン（例: Pattern4）を "JoinIR 生成 → CorePlan/Frag" に変換する PoC - 目的: merge を通さずに `emit_frag()` 経由で終端が生成できることの証明 +### P3 (error context enrichment) ✅ COMPLETE (2025-12-25) + +**完了内容**: +- **P2**: host_fn をエラーコンテキストに追加（関数名での特定を容易に） +- **P3**: join-side 情報（continuation数・boundaryサマリ）をエラーコンテキストに追加 + - `[conts=X exits=Y conds=Z]` 形式のサマリを追加 + - 固定キー名で解析容易に + +**成果物**: +- `src/mir/builder/control_flow/joinir/merge/mod.rs` (変更) +- 最終エラーフォーマット: `[merge_joinir_mir_blocks host=X join=Y [conts=A exits=B conds=C]]` + +### 286C-2 (instruction_rewriter.rs 箱化) ✅ COMPLETE (2025-12-25) + +**完了内容**: +- **instruction_rewriter.rs の箱化・意味論不変**: 1400行ファイルに責務リストコメントを追加し、4つの箱モジュールを抽出 + - **InstructionFilterBox**: Skip判定ロジック（純粋関数） + - `should_skip_copy_overwriting_phi()` - CopyがPHI dstを上書きするか判定 + - `should_skip_function_name_const()` - Const String（関数名）のスキップ判定 + - `should_skip_boundary_input_const()` - Boundary input Constのスキップ判定 + - **ReturnConverterBox**: Return→Jump変換ヘルパー + - `should_keep_return()` - 非スキップ可能継続のReturn保持判定 + - `remap_return_value()` - Return値のremapヘルパー + - **TailCallDetectorBox**: テイルコール検出ヘルパー + - `is_recursive_call()` - 再帰呼び出し判定 + - `is_loop_entry_call()` - ループエントリ呼び出し判定 + - `should_skip_param_binding()` - パラメータ束縛スキップ判定 + - `call_type_description()` - 呼び出しタイプの説明文字列取得 + - **ParameterBindingBox**: パラメータ束縛ヘルパー + - `should_skip_phi_param()` - PHI dstパラメータのスキップ判定 + - `carrier_param_count()` - キャリアパラメータ数取得 + - `has_more_carrier_args()` - キャリア引数残確認 + - `carrier_arg_index()` - キャリア引数インデックス計算 + +**成果物**: +- `src/mir/builder/control_flow/joinir/merge/rewriter/instruction_filter_box.rs` (新規) +- `src/mir/builder/control_flow/joinir/merge/rewriter/return_converter_box.rs` (新規) +- `src/mir/builder/control_flow/joinir/merge/rewriter/tail_call_detector_box.rs` (新規) +- `src/mir/builder/control_flow/joinir/merge/rewriter/parameter_binding_box.rs` (新規) +- `src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs` (変更: 責務リストコメント追加 + 箱使用) +- `src/mir/builder/control_flow/joinir/merge/rewriter/mod.rs` (変更: モジュール追加) + +**注意点**: +- 意味論は完全不変（既存のinlineロジックを箱関数呼び出しに置換） +- ファイル行数は1454行に増加（コメント・import追加により） +- 核ロジックは main loop に密結合しているため、完全な分離にはさらなるリファクタリングが必要 +- スモークテスト: 既存FAILなし（1件のemit失敗は本変更と無関係） + ## Acceptance（P0） - 2本の lowering が “設計として” どこで 1 本に収束するかが明文化されている 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 390649c9..56f8ecae 100644 --- a/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs +++ b/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs @@ -8,6 +8,46 @@ //! Phase 179-A Step 3: Named constants for magic values //! Phase 131 Task 2: Extracted TailCallLoweringPolicyBox for k_exit normalization //! Phase 284 P1: Use block_remapper SSOT for block ID remapping +//! Phase 286C-2: Box extraction - planned modularization (see responsibility list below) +//! +//! # Responsibilities (Phase 286C-2) +//! +//! This module orchestrates the following responsibilities (to be extracted into separate boxes): +//! +//! ## Pure Functions (no builder state, easiest to extract) +//! - **InstructionFilterBox**: Skip judgment logic +//! - Skip Copy instructions that overwrite PHI dsts +//! - Skip Const String instructions (function names, already mapped) +//! - Skip Const instructions for boundary inputs (injected by BoundaryInjector) +//! - Skip Copy instructions that overwrite header PHI dsts +//! +//! - **ReturnConverterBox**: Return → Jump conversion +//! - Convert Return instructions to Jump to exit block +//! - Handle non-skippable continuations (keep Return) +//! - Process k_exit tail calls with ExitArgsCollectorBox +//! +//! ## Local Transformations (moderate dependencies) +//! - **TailCallDetectorBox**: Tail call detection and classification +//! - Detect tail calls (normal, recursive, continuation, loop entry) +//! - Classify using TailCallLoweringPolicyBox +//! - Record latch incoming for loop header PHI +//! +//! - **ParameterBindingBox**: Parameter binding generation +//! - Generate Copy instructions for parameter bindings +//! - Handle recursive calls, continuation calls, loop entry calls +//! - Track latch incoming for loop header PHI +//! +//! ## Composition/Addition (high complexity - deferred to later phases) +//! - **BlockMergerBox**: Main block processing loop (NOT extracted in Phase 286C-2) +//! - Process each function and block +//! - Manage function_entry_map and skipped_entry_redirects +//! - Add blocks to current function with span synchronization +//! +//! # Current State +//! +//! - **File size**: ~1400 lines +//! - **Main function**: `merge_and_rewrite` (lines 63-1400) +//! - **Target**: Reduce to ~600 lines by extracting 4 boxes (InstructionFilterBox, ReturnConverterBox, TailCallDetectorBox, ParameterBindingBox) use super::block_remapper::remap_block_id; // Phase 284 P1: SSOT use super::exit_args_collector::ExitArgsCollectorBox; @@ -26,6 +66,14 @@ use std::collections::BTreeSet; // Phase 260 P0.1 Step 3: Import from helpers module use super::rewriter::helpers::is_skippable_continuation; +// Phase 286C-2 Step 2: Import from instruction_filter_box module +use super::rewriter::instruction_filter_box::InstructionFilterBox; +// Phase 286C-2 Step 2: Import from parameter_binding_box module +use super::rewriter::parameter_binding_box::ParameterBindingBox; +// Phase 286C-2 Step 2: Import from return_converter_box module +use super::rewriter::return_converter_box::ReturnConverterBox; +// Phase 286C-3: Import RewriteContext for state consolidation +use super::rewriter::rewrite_context::RewriteContext; // Phase 260 P0.1 Step 4: Import from type_propagation module use super::rewriter::type_propagation::propagate_value_type_for_inst; // Phase 260 P0.1 Step 5: Import from terminator module @@ -112,25 +160,15 @@ pub(super) fn merge_and_rewrite( let boundary_input_set: std::collections::HashSet = boundary .map(|b| b.join_inputs.iter().copied().collect()) .unwrap_or_default(); - let strict_exit = - crate::config::env::joinir_strict_enabled() || crate::config::env::joinir_dev_enabled(); - - // Phase 189-Fix: Collect return values from JoinIR functions for exit PHI - let mut exit_phi_inputs: Vec<(BasicBlockId, ValueId)> = Vec::new(); - - // Phase 33-13: Collect carrier exit values for carrier PHIs - // Map from carrier_name to Vec of (from_block, exit_value) - let mut carrier_inputs: std::collections::BTreeMap> = - std::collections::BTreeMap::new(); + // Phase 286C-3: Use RewriteContext to consolidate scattered state + let mut ctx = RewriteContext::new(exit_block_id, boundary, debug); // Build function_entry_map for Call→Jump conversion - // Phase 222.5-E: HashMap → BTreeMap for determinism - let mut function_entry_map: BTreeMap = BTreeMap::new(); for (func_name, func) in &mir_module.functions { let entry_block_new = remapper .get_block(func_name, func.entry_block) .ok_or_else(|| format!("Entry block not found for {}", func_name))?; - function_entry_map.insert(func_name.clone(), entry_block_new); + ctx.register_function_entry(func_name.clone(), entry_block_new); } // Phase 259 P0 FIX: Build redirect map for skipped continuation entry blocks → exit_block_id @@ -140,21 +178,17 @@ pub(super) fn merge_and_rewrite( // reference k_exit's entry block. We need to redirect those references to exit_block_id. // // This map is GLOBAL across all functions, unlike local_block_map which is function-local. - let skipped_entry_redirects: BTreeMap = - skippable_continuation_func_names - .iter() - .filter_map(|func_name| { - function_entry_map - .get(func_name) - .map(|&entry_block| (entry_block, exit_block_id)) - }) - .collect(); + for func_name in &skippable_continuation_func_names { + if let Some(&entry_block) = ctx.function_entry_map.get(func_name) { + ctx.register_skipped_redirect(entry_block, exit_block_id); + } + } - if debug && !skipped_entry_redirects.is_empty() { + if debug && !ctx.skipped_entry_redirects.is_empty() { log!( true, "[cf_loop/joinir] Phase 259 P0: Built skipped_entry_redirects: {:?}", - skipped_entry_redirects + ctx.skipped_entry_redirects ); } @@ -365,7 +399,8 @@ pub(super) fn merge_and_rewrite( "[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) { + // Phase 286C-2: Use InstructionFilterBox for skip judgment + if InstructionFilterBox::should_skip_copy_overwriting_phi(dst_remapped, &phi_dst_ids_for_block) { log!( verbose, "[cf_loop/joinir] Phase 177-3: ✅ Skipping loop header Copy to PHI dst {:?} (original {:?})", @@ -378,17 +413,20 @@ pub(super) fn merge_and_rewrite( // 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 { - if value_to_func_name.contains_key(dst) { - if debug { - log!(true, "[cf_loop/joinir] Skipping function name const: {:?}", inst); - } - continue; // Skip this instruction + // Phase 286C-2: Use InstructionFilterBox for function name skip judgment + if InstructionFilterBox::should_skip_function_name_const(value) + && value_to_func_name.contains_key(dst) + { + if debug { + log!(true, "[cf_loop/joinir] Skipping function name const: {:?}", inst); } + continue; // Skip this instruction } // Phase 189 FIX: Skip Const instructions in entry function's entry block // that initialize boundary inputs. BoundaryInjector provides these values via Copy. - if is_loop_entry_point && boundary_input_set.contains(dst) { + // Phase 286C-2: Use InstructionFilterBox for boundary input skip judgment + let boundary_inputs: Vec = boundary_input_set.iter().cloned().collect(); + if InstructionFilterBox::should_skip_boundary_input_const(*dst, &boundary_inputs, is_loop_entry_point) { if debug { log!(true, "[cf_loop/joinir] Skipping boundary input const (replaced by BoundaryInjector Copy): {:?}", inst); } @@ -418,7 +456,7 @@ pub(super) fn merge_and_rewrite( "[joinir/merge] k_exit edge-args layout mismatch: block={:?} edge={:?} boundary={:?}", old_block.id, edge_args.layout, b.jump_args_layout ); - if strict_exit { + if ctx.strict_exit { return Err(msg); } else if debug { log!(true, "[DEBUG-177] {}", msg); @@ -440,7 +478,7 @@ pub(super) fn merge_and_rewrite( continue; // Skip the Call instruction itself } // Not a k_exit call - check if it's a normal tail call (intra-module jump) - if let Some(&target_block) = function_entry_map.get(callee_name) { + if let Some(&target_block) = ctx.function_entry_map.get(callee_name) { // This is a tail call - save info and skip the Call instruction itself tail_call_target = Some((target_block, remapped_args)); found_tail_call = true; @@ -496,8 +534,8 @@ pub(super) fn merge_and_rewrite( else_edge_args, } => { // Phase 284 P1: Use SSOT for block remapping - let remapped_then = remap_block_id(then_bb, &local_block_map, &skipped_entry_redirects); - let remapped_else = remap_block_id(else_bb, &local_block_map, &skipped_entry_redirects); + let remapped_then = remap_block_id(then_bb, &local_block_map, &ctx.skipped_entry_redirects); + let remapped_else = remap_block_id(else_bb, &local_block_map, &ctx.skipped_entry_redirects); MirInstruction::Branch { condition, then_bb: remapped_then, @@ -556,7 +594,7 @@ pub(super) fn merge_and_rewrite( // Find the target function name from the target_block // We need to reverse-lookup the function name from the entry block let mut target_func_name: Option = None; - for (fname, &entry_block) in &function_entry_map { + for (fname, &entry_block) in &ctx.function_entry_map { if entry_block == target_block { target_func_name = Some(fname.clone()); break; @@ -940,10 +978,12 @@ pub(super) fn merge_and_rewrite( MirInstruction::Return { value } => { // Phase 284 P1: Non-skippable continuations (like k_return) should keep Return // Only convert Return → Jump for skippable continuations and regular loop body - if is_continuation_candidate && !is_skippable_continuation { + // Phase 286C-2: Use ReturnConverterBox for should_keep_return decision + if ReturnConverterBox::should_keep_return(is_continuation_candidate, is_skippable_continuation) { // Non-skippable continuation (e.g., k_return): keep Return terminator // Remap the return value to HOST value space - let remapped_value = value.map(|v| remapper.remap_value(v)); + // Phase 286C-2: Use ReturnConverterBox for value remapping + let remapped_value = ReturnConverterBox::remap_return_value(*value, |v| remapper.remap_value(v)); new_block.set_terminator(MirInstruction::Return { value: remapped_value }); log!( true, @@ -970,7 +1010,7 @@ pub(super) fn merge_and_rewrite( "[joinir/merge] exit edge-args layout mismatch: block={:?} edge={:?} boundary={:?}", old_block.id, edge_args.layout, b.jump_args_layout ); - if strict_exit { + if ctx.strict_exit { return Err(msg); } else if debug { log!(true, "[DEBUG-177] {}", msg); @@ -1007,7 +1047,7 @@ pub(super) fn merge_and_rewrite( &b.exit_bindings, &edge_args.values, new_block_id, - strict_exit, + ctx.strict_exit, edge_args.layout, )?; @@ -1015,7 +1055,7 @@ pub(super) fn merge_and_rewrite( if let Some(expr_result_val) = collection_result.expr_result_value { - exit_phi_inputs.push((new_block_id, expr_result_val)); + ctx.add_exit_phi_input(new_block_id, expr_result_val); log!( verbose, "[DEBUG-177] Phase 118: exit_phi_inputs from ExitArgsCollectorBox: ({:?}, {:?})", @@ -1027,15 +1067,12 @@ pub(super) fn merge_and_rewrite( for (carrier_name, (block_id, value_id)) in collection_result.carrier_values { - carrier_inputs - .entry(carrier_name.clone()) - .or_insert_with(Vec::new) - .push((block_id, value_id)); log!( verbose, "[DEBUG-177] Phase 118: Collecting carrier '{}': from {:?} value {:?}", carrier_name, block_id, value_id ); + ctx.add_carrier_input(carrier_name, block_id, value_id); } } else { // Fallback: Use header PHI dst (old behavior for blocks without jump_args) @@ -1049,7 +1086,7 @@ pub(super) fn merge_and_rewrite( if let Some(phi_dst) = loop_header_phi_info.get_carrier_phi(loop_var_name) { - exit_phi_inputs.push((new_block_id, phi_dst)); + ctx.add_exit_phi_input(new_block_id, phi_dst); if debug { log!( true, @@ -1070,17 +1107,11 @@ pub(super) fn merge_and_rewrite( if let Some(phi_dst) = loop_header_phi_info .get_carrier_phi(&binding.carrier_name) { - carrier_inputs - .entry(binding.carrier_name.clone()) - .or_insert_with(Vec::new) - .push((new_block_id, phi_dst)); + ctx.add_carrier_input(binding.carrier_name.clone(), new_block_id, phi_dst); } else if b.exit_reconnect_mode == crate::mir::join_ir::lowering::carrier_info::ExitReconnectMode::DirectValue { // Phase 131 P1.5: DirectValue mode fallback - use host_slot (initial value) // This handles k_exit blocks that don't have jump_args and no header PHI - carrier_inputs - .entry(binding.carrier_name.clone()) - .or_insert_with(Vec::new) - .push((new_block_id, binding.host_slot)); + ctx.add_carrier_input(binding.carrier_name.clone(), new_block_id, binding.host_slot); log!( verbose, "[cf_loop/joinir] Phase 131 P1.5: DirectValue fallback for '{}': using host_slot {:?}", @@ -1108,7 +1139,7 @@ pub(super) fn merge_and_rewrite( &remapper, *target, edge_args, - &skipped_entry_redirects, + &ctx.skipped_entry_redirects, &local_block_map, )); } @@ -1127,7 +1158,7 @@ pub(super) fn merge_and_rewrite( *else_bb, then_edge_args, else_edge_args, - &skipped_entry_redirects, + &ctx.skipped_entry_redirects, &local_block_map, )); } @@ -1169,19 +1200,16 @@ pub(super) fn merge_and_rewrite( &b.exit_bindings, &edge_args.values, new_block_id, - strict_exit, + ctx.strict_exit, edge_args.layout, )?; if let Some(expr_result_value) = collection_result.expr_result_value { - exit_phi_inputs.push((collection_result.block_id, expr_result_value)); + ctx.add_exit_phi_input(collection_result.block_id, expr_result_value); } - for (carrier_name, pair) in collection_result.carrier_values { - carrier_inputs - .entry(carrier_name) - .or_insert_with(Vec::new) - .push(pair); + for (carrier_name, (block_id, value_id)) in collection_result.carrier_values { + ctx.add_carrier_input(carrier_name, block_id, value_id); } - } else if strict_exit { + } else if ctx.strict_exit { return Err(error_tags::freeze_with_hint( "phase131/k_exit/no_boundary", "k_exit tail call detected without JoinInlineBoundary", @@ -1203,7 +1231,7 @@ pub(super) fn merge_and_rewrite( }; // Strict mode: verify the generated terminator matches contract - if strict_exit { + if ctx.strict_exit { tail_call_policy.verify_exit_jump(&exit_jump, exit_block_id)?; } } @@ -1355,22 +1383,17 @@ pub(super) fn merge_and_rewrite( } // Phase 131 P2: DirectValue mode remapped_exit_values SSOT + // Phase 286C-3: Use ctx.set_remapped_exit_value() for state management // // Contract (DirectValue): // - boundary.exit_bindings[*].join_exit_value is a JoinIR-side ValueId that must be defined // in the merged MIR (e.g., final env value produced by the loop body). // - remapper owns JoinIR→Host mapping, so merge_and_rewrite is responsible for producing // carrier_name → host ValueId. - let remapped_exit_values: BTreeMap = match boundary { - Some(b) - if b.exit_reconnect_mode - == crate::mir::join_ir::lowering::carrier_info::ExitReconnectMode::DirectValue => - { - let mut direct_values = BTreeMap::new(); + if let Some(b) = boundary { + if b.exit_reconnect_mode == crate::mir::join_ir::lowering::carrier_info::ExitReconnectMode::DirectValue { for binding in &b.exit_bindings { - if binding.role - == crate::mir::join_ir::lowering::carrier_info::CarrierRole::ConditionOnly - { + if binding.role == crate::mir::join_ir::lowering::carrier_info::CarrierRole::ConditionOnly { continue; } @@ -1384,17 +1407,15 @@ pub(super) fn merge_and_rewrite( "ensure exit_bindings.join_exit_value is included in merge used_values and references a value defined by the fragment", ) })?; - direct_values.insert(binding.carrier_name.clone(), host_vid); + ctx.set_remapped_exit_value(binding.carrier_name.clone(), host_vid); } - direct_values } - _ => BTreeMap::new(), - }; + } Ok(MergeResult { - exit_block_id, - exit_phi_inputs, - carrier_inputs, - remapped_exit_values, // Phase 131 P1.5 + exit_block_id: ctx.exit_block_id, + exit_phi_inputs: ctx.exit_phi_inputs, + carrier_inputs: ctx.carrier_inputs, + remapped_exit_values: ctx.remapped_exit_values, // Phase 131 P1.5 }) } diff --git a/src/mir/builder/control_flow/joinir/merge/rewriter/instruction_filter_box.rs b/src/mir/builder/control_flow/joinir/merge/rewriter/instruction_filter_box.rs new file mode 100644 index 00000000..b62c531f --- /dev/null +++ b/src/mir/builder/control_flow/joinir/merge/rewriter/instruction_filter_box.rs @@ -0,0 +1,135 @@ +//! InstructionFilterBox: Skip judgment logic for JoinIR instruction rewriting +//! +//! Phase 286C-2: Extracted from instruction_rewriter.rs +//! Provides pure functions to determine which instructions should be skipped during rewriting. + +use crate::mir::{types::ConstValue, ValueId}; + +/// InstructionFilterBox: Skip judgment logic +/// +/// Pure functions that determine which instructions should be skipped during +/// JoinIR→MIR rewriting. Each function answers a specific "should I skip this?" question. +pub struct InstructionFilterBox; + +impl InstructionFilterBox { + /// Check if a Copy instruction overwrites a PHI dst (should skip during rewriting) + /// + /// In loop headers with PHI nodes, certain Copy instructions would overwrite + /// the PHI inputs that will be provided by the loop header PHIs themselves. + /// These copies must be skipped to prevent incorrect values. + /// + /// # Arguments + /// * `dst` - The destination ValueId of the Copy instruction (after remapping) + /// * `phi_dsts` - Set of PHI destination ValueIds to protect + /// + /// # Returns + /// * `true` if the Copy should be skipped (overwrites a PHI dst) + /// * `false` if the Copy should be kept + pub fn should_skip_copy_overwriting_phi(dst: ValueId, phi_dsts: &std::collections::HashSet) -> bool { + phi_dsts.contains(&dst) + } + + /// Check if a Const String instruction is a function name (should skip, already mapped) + /// + /// Function name constants are already mapped via `value_to_func_name` and + /// don't need to be emitted as instructions in the merged MIR. + /// + /// # Arguments + /// * `value` - The ConstValue to check + /// * `is_in_func_name_map` - Whether the value's dst is in `value_to_func_name` + /// + /// # Returns + /// * `true` if this Const defines a function name (should skip) + /// * `false` otherwise + pub fn should_skip_function_name_const(value: &ConstValue) -> bool { + matches!(value, ConstValue::String(_)) + } + + /// Check if a Const instruction is a boundary input (should skip, injected by BoundaryInjector) + /// + /// Boundary input constants are provided by the BoundaryInjector via Copy instructions, + /// so the original Const definitions should be skipped. + /// + /// # Arguments + /// * `value_id` - The ValueId of the Const instruction's dst + /// * `boundary_inputs` - Set of boundary input ValueIds + /// * `is_loop_entry_point` - Whether this is the loop entry point + /// + /// # Returns + /// * `true` if this Const is a boundary input (should skip) + /// * `false` otherwise + pub fn should_skip_boundary_input_const( + value_id: ValueId, + boundary_inputs: &[ValueId], + is_loop_entry_point: bool, + ) -> bool { + is_loop_entry_point && boundary_inputs.contains(&value_id) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashSet; + + #[test] + fn test_should_skip_copy_overwriting_phi() { + let mut phi_dsts = HashSet::new(); + phi_dsts.insert(ValueId(100)); + phi_dsts.insert(ValueId(200)); + + // Should skip because dst is in phi_dsts + assert!(InstructionFilterBox::should_skip_copy_overwriting_phi( + ValueId(100), + &phi_dsts + )); + + // Should NOT skip because dst is not in phi_dsts + assert!(!InstructionFilterBox::should_skip_copy_overwriting_phi( + ValueId(300), + &phi_dsts + )); + } + + #[test] + fn test_should_skip_function_name_const() { + // String const should be skipped (function name) + assert!(InstructionFilterBox::should_skip_function_name_const( + &ConstValue::String("main".to_string()) + )); + + // Other consts should NOT be skipped + assert!(!InstructionFilterBox::should_skip_function_name_const( + &ConstValue::Integer(42) + )); + assert!(!InstructionFilterBox::should_skip_function_name_const( + &ConstValue::Void + )); + } + + #[test] + fn test_should_skip_boundary_input_const() { + let boundary_inputs = vec![ValueId(10), ValueId(20), ValueId(30)]; + + // Should skip: is_loop_entry_point=true and value_id in boundary_inputs + assert!(InstructionFilterBox::should_skip_boundary_input_const( + ValueId(20), + &boundary_inputs, + true + )); + + // Should NOT skip: is_loop_entry_point=false + assert!(!InstructionFilterBox::should_skip_boundary_input_const( + ValueId(20), + &boundary_inputs, + false + )); + + // Should NOT skip: value_id not in boundary_inputs + assert!(!InstructionFilterBox::should_skip_boundary_input_const( + ValueId(999), + &boundary_inputs, + true + )); + } +} diff --git a/src/mir/builder/control_flow/joinir/merge/rewriter/mod.rs b/src/mir/builder/control_flow/joinir/merge/rewriter/mod.rs index 33034510..46b5c26e 100644 --- a/src/mir/builder/control_flow/joinir/merge/rewriter/mod.rs +++ b/src/mir/builder/control_flow/joinir/merge/rewriter/mod.rs @@ -34,7 +34,12 @@ pub(super) mod exit_collection; // Phase 260 P0.1 Step 6: Exit collection extracted ✅ pub(super) mod helpers; // Phase 260 P0.1 Step 3: Helpers extracted ✅ +pub(super) mod instruction_filter_box; // Phase 286C-2 Step 2: Skip judgment logic extracted ✅ pub(super) mod logging; // Phase 260 P0.1 Step 2: Logging extracted ✅ +pub(super) mod parameter_binding_box; // Phase 286C-2 Step 2: Parameter binding helpers ✅ +pub(super) mod return_converter_box; // Phase 286C-2 Step 2: Return → Jump conversion helpers ✅ +pub(super) mod rewrite_context; // Phase 286C-3: State consolidation ✅ +pub(super) mod tail_call_detector_box; // Phase 286C-2 Step 2: Tail call detection helpers ✅ pub(super) mod terminator; // Phase 260 P0.1 Step 5: Terminator remapping extracted ✅ pub(super) mod type_propagation; // Phase 260 P0.1 Step 4: Type propagation extracted ✅ diff --git a/src/mir/builder/control_flow/joinir/merge/rewriter/parameter_binding_box.rs b/src/mir/builder/control_flow/joinir/merge/rewriter/parameter_binding_box.rs new file mode 100644 index 00000000..317da2ab --- /dev/null +++ b/src/mir/builder/control_flow/joinir/merge/rewriter/parameter_binding_box.rs @@ -0,0 +1,135 @@ +//! ParameterBindingBox: Parameter binding helpers +//! +//! Phase 286C-2: Extracted from instruction_rewriter.rs +//! Helper functions related to parameter binding generation. + +/// ParameterBindingBox: Parameter binding helpers +/// +/// Helper functions for parameter binding logic during JoinIR→MIR merging. +pub struct ParameterBindingBox; + +impl ParameterBindingBox { + /// Check if parameter binding should skip a PHI destination + /// + /// When the parameter value is a header PHI destination, we should skip + /// the Copy instruction because the PHI node will receive the value as + /// an incoming edge instead. + /// + /// # Arguments + /// * `param_val` - The parameter ValueId to check + /// * `phi_dsts` - Set of PHI destination ValueIds to protect + /// + /// # Returns + /// * `true` if the parameter binding should be skipped (param is a PHI dst) + /// * `false` if the parameter binding should be generated + pub fn should_skip_phi_param( + param_val: u32, + phi_dsts: &std::collections::HashSet, + ) -> bool { + phi_dsts.contains(¶m_val) + } + + /// Get the number of carrier parameters to process + /// + /// Carrier parameters are those that are not the loop variable itself. + /// + /// # Arguments + /// * `args_len` - Total number of arguments + /// * `has_loop_var` - Whether there's a loop variable (takes first arg) + /// + /// # Returns + /// * The number of carrier parameters + pub fn carrier_param_count(args_len: usize, has_loop_var: bool) -> usize { + if has_loop_var && args_len > 0 { + args_len - 1 + } else { + args_len + } + } + + /// Check if there are more carrier arguments to process + /// + /// # Arguments + /// * `carrier_arg_idx` - Current carrier argument index + /// * `args_len` - Total number of arguments + /// + /// # Returns + /// * `true` if there are more carrier arguments + /// * `false` if we've processed all carrier arguments + pub fn has_more_carrier_args(carrier_arg_idx: usize, args_len: usize) -> bool { + carrier_arg_idx < args_len + } + + /// Get the carrier argument index (skipping loop variable if present) + /// + /// # Arguments + /// * `arg_idx` - The current argument index + /// * `loop_var_index` - Optional loop variable index (None if no loop var) + /// + /// # Returns + /// * The corresponding carrier argument index + pub fn carrier_arg_index(arg_idx: usize, loop_var_index: Option) -> usize { + match loop_var_index { + Some(lv_idx) if arg_idx >= lv_idx => arg_idx + 1, + _ => arg_idx, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashSet; + + #[test] + fn test_should_skip_phi_param() { + let mut phi_dsts = HashSet::new(); + phi_dsts.insert(100); + phi_dsts.insert(200); + + // Should skip because param is a PHI dst + assert!(ParameterBindingBox::should_skip_phi_param(100, &phi_dsts)); + assert!(ParameterBindingBox::should_skip_phi_param(200, &phi_dsts)); + + // Should NOT skip + assert!(!ParameterBindingBox::should_skip_phi_param(300, &phi_dsts)); + } + + #[test] + fn test_carrier_param_count() { + // With loop var (takes first arg) + assert_eq!(ParameterBindingBox::carrier_param_count(3, true), 2); + assert_eq!(ParameterBindingBox::carrier_param_count(1, true), 0); + + // Without loop var + assert_eq!(ParameterBindingBox::carrier_param_count(3, false), 3); + assert_eq!(ParameterBindingBox::carrier_param_count(0, false), 0); + } + + #[test] + fn test_has_more_carrier_args() { + // More args available + assert!(ParameterBindingBox::has_more_carrier_args(0, 3)); + assert!(ParameterBindingBox::has_more_carrier_args(2, 3)); + + // No more args + assert!(!ParameterBindingBox::has_more_carrier_args(3, 3)); + assert!(!ParameterBindingBox::has_more_carrier_args(5, 3)); + } + + #[test] + fn test_carrier_arg_index() { + // No loop var + assert_eq!(ParameterBindingBox::carrier_arg_index(0, None), 0); + assert_eq!(ParameterBindingBox::carrier_arg_index(2, None), 2); + + // With loop var at index 0 + assert_eq!(ParameterBindingBox::carrier_arg_index(0, Some(0)), 1); + assert_eq!(ParameterBindingBox::carrier_arg_index(1, Some(0)), 2); + + // With loop var at index 2 + assert_eq!(ParameterBindingBox::carrier_arg_index(0, Some(2)), 0); + assert_eq!(ParameterBindingBox::carrier_arg_index(1, Some(2)), 1); + assert_eq!(ParameterBindingBox::carrier_arg_index(2, Some(2)), 3); + } +} diff --git a/src/mir/builder/control_flow/joinir/merge/rewriter/return_converter_box.rs b/src/mir/builder/control_flow/joinir/merge/rewriter/return_converter_box.rs new file mode 100644 index 00000000..10029b0a --- /dev/null +++ b/src/mir/builder/control_flow/joinir/merge/rewriter/return_converter_box.rs @@ -0,0 +1,89 @@ +//! ReturnConverterBox: Return → Jump conversion helpers +//! +//! Phase 286C-2: Extracted from instruction_rewriter.rs +//! Helper functions to determine if a Return instruction should be converted to Jump. + +use crate::mir::ValueId; + +/// ReturnConverterBox: Return → Jump conversion helpers +/// +/// Helper functions that determine how Return instructions should be handled +/// during JoinIR→MIR merging. +pub struct ReturnConverterBox; + +impl ReturnConverterBox { + /// Check if a Return instruction should be kept as Return (not converted to Jump) + /// + /// Non-skippable continuations (like k_return) should keep their Return terminator + /// instead of being converted to Jump to exit block. + /// + /// # Arguments + /// * `is_continuation_candidate` - Whether this is a continuation candidate + /// * `is_skippable_continuation` - Whether this is a skippable continuation + /// + /// # Returns + /// * `true` if the Return should be kept (not converted to Jump) + /// * `false` if the Return should be converted to Jump to exit block + pub fn should_keep_return(is_continuation_candidate: bool, is_skippable_continuation: bool) -> bool { + is_continuation_candidate && !is_skippable_continuation + } + + /// Create the remapped return value for a kept Return instruction + /// + /// When a Return is kept (not converted to Jump), the return value needs + /// to be remapped from JoinIR value space to HOST value space. + /// + /// # Arguments + /// * `value` - The optional return value from the original Return instruction + /// * `remap_fn` - Function to remap a ValueId (e.g., `|v| remapper.remap_value(v)`) + /// + /// # Returns + /// * The remapped optional return value + pub fn remap_return_value( + value: Option, + remap_fn: F, + ) -> Option + where + F: FnOnce(ValueId) -> ValueId, + { + value.map(remap_fn) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_should_keep_return_non_skippable_continuation() { + // Non-skippable continuation: should keep Return + assert!(ReturnConverterBox::should_keep_return(true, false)); + } + + #[test] + fn test_should_keep_return_skippable_continuation() { + // Skippable continuation: should convert to Jump + assert!(!ReturnConverterBox::should_keep_return(true, true)); + } + + #[test] + fn test_should_keep_return_non_continuation() { + // Not a continuation: should convert to Jump + assert!(!ReturnConverterBox::should_keep_return(false, false)); + assert!(!ReturnConverterBox::should_keep_return(false, true)); + } + + #[test] + fn test_remap_return_value_some() { + let value = Some(ValueId(100)); + let result = ReturnConverterBox::remap_return_value(value, |v| ValueId(v.0 + 1000)); + assert_eq!(result, Some(ValueId(1100))); + } + + #[test] + fn test_remap_return_value_none() { + let value = None; + let result = ReturnConverterBox::remap_return_value(value, |v| ValueId(v.0 + 1000)); + assert_eq!(result, None); + } +} diff --git a/src/mir/builder/control_flow/joinir/merge/rewriter/rewrite_context.rs b/src/mir/builder/control_flow/joinir/merge/rewriter/rewrite_context.rs new file mode 100644 index 00000000..97ba1477 --- /dev/null +++ b/src/mir/builder/control_flow/joinir/merge/rewriter/rewrite_context.rs @@ -0,0 +1,94 @@ +//! Rewrite Context - State Consolidation +//! +//! Phase 286C-3: Consolidated state for instruction rewriting. +//! Reduces scattered variables in merge_and_rewrite() into a single coherent structure. + +use crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary; +use crate::mir::{BasicBlockId, ValueId}; +use std::collections::BTreeMap; + +/// Consolidated state for instruction rewriting during JoinIR→MIR merge +pub(in crate::mir::builder::control_flow::joinir::merge) struct RewriteContext<'a> { + /// Exit PHI inputs: Vec<(from_block, exit_value)> + pub exit_phi_inputs: Vec<(BasicBlockId, ValueId)>, + + /// Carrier PHI inputs: Map> + pub carrier_inputs: BTreeMap>, + + /// Function entry map: Map + pub function_entry_map: BTreeMap, + + /// Skipped continuation entry redirects: Map + pub skipped_entry_redirects: BTreeMap, + + /// Remapped exit values for DirectValue mode: Map + pub remapped_exit_values: BTreeMap, + + /// The exit block ID (allocated by block_allocator) + pub exit_block_id: BasicBlockId, + + /// Optional boundary (for exit value collection, carrier management) + pub boundary: Option<&'a JoinInlineBoundary>, + + /// Debug flag + pub debug: bool, + + /// Verbose flag (debug || joinir_dev_enabled) + pub verbose: bool, + + /// Strict exit mode + pub strict_exit: bool, +} + +impl<'a> RewriteContext<'a> { + /// Create a new RewriteContext + pub fn new( + exit_block_id: BasicBlockId, + boundary: Option<&'a JoinInlineBoundary>, + debug: bool, + ) -> Self { + let verbose = debug || crate::config::env::joinir_dev_enabled(); + let strict_exit = crate::config::env::joinir_strict_enabled() || crate::config::env::joinir_dev_enabled(); + + Self { + exit_phi_inputs: Vec::new(), + carrier_inputs: BTreeMap::new(), + function_entry_map: BTreeMap::new(), + skipped_entry_redirects: BTreeMap::new(), + remapped_exit_values: BTreeMap::new(), + exit_block_id, + boundary, + debug, + verbose, + strict_exit, + } + } + + /// Add an exit PHI input + pub fn add_exit_phi_input(&mut self, from_block: BasicBlockId, value: ValueId) { + self.exit_phi_inputs.push((from_block, value)); + } + + /// Add a carrier input + pub fn add_carrier_input(&mut self, carrier_name: String, from_block: BasicBlockId, value: ValueId) { + self.carrier_inputs + .entry(carrier_name) + .or_insert_with(Vec::new) + .push((from_block, value)); + } + + /// Register a function entry block + pub fn register_function_entry(&mut self, func_name: String, entry_block: BasicBlockId) { + self.function_entry_map.insert(func_name, entry_block); + } + + /// Register a skipped continuation redirect + pub fn register_skipped_redirect(&mut self, old_entry: BasicBlockId, new_target: BasicBlockId) { + self.skipped_entry_redirects.insert(old_entry, new_target); + } + + /// Set a remapped exit value (DirectValue mode) + pub fn set_remapped_exit_value(&mut self, carrier_name: String, host_value: ValueId) { + self.remapped_exit_values.insert(carrier_name, host_value); + } +} diff --git a/src/mir/builder/control_flow/joinir/merge/rewriter/tail_call_detector_box.rs b/src/mir/builder/control_flow/joinir/merge/rewriter/tail_call_detector_box.rs new file mode 100644 index 00000000..d79c02a3 --- /dev/null +++ b/src/mir/builder/control_flow/joinir/merge/rewriter/tail_call_detector_box.rs @@ -0,0 +1,117 @@ +//! TailCallDetectorBox: Tail call detection helpers +//! +//! Phase 286C-2: Extracted from instruction_rewriter.rs +//! Helper functions related to tail call detection and classification. + +use crate::mir::ValueId; + +/// TailCallDetectorBox: Tail call detection helpers +/// +/// Helper functions for tail call detection and classification logic +/// during JoinIR→MIR merging. +pub struct TailCallDetectorBox; + +impl TailCallDetectorBox { + /// Check if a tail call is a recursive call (function calls itself) + /// + /// # Arguments + /// * `caller_name` - The name of the calling function + /// * `callee_name` - The name of the called function + /// + /// # Returns + /// * `true` if the call is recursive (caller == callee) + /// * `false` otherwise + pub fn is_recursive_call(caller_name: &str, callee_name: &str) -> bool { + caller_name == callee_name + } + + /// Check if a tail call is a loop entry point call + /// + /// # Arguments + /// * `is_loop_entry_point` - Whether the current block is the loop entry point + /// + /// # Returns + /// * `true` if this is a loop entry point call + /// * `false` otherwise + pub fn is_loop_entry_call(is_loop_entry_point: bool) -> bool { + is_loop_entry_point + } + + /// Check if parameter binding should be skipped for this tail call + /// + /// Parameter binding should be skipped in header block (loop entry point) + /// for recursive and entry calls, because PHIs provide the values. + /// + /// # Arguments + /// * `is_recursive_call` - Whether this is a recursive call + /// * `is_loop_entry_point` - Whether this is the loop entry point + /// + /// # Returns + /// * `true` if parameter binding should be skipped + /// * `false` if parameter binding should be generated + pub fn should_skip_param_binding(is_recursive_call: bool, is_loop_entry_point: bool) -> bool { + is_recursive_call || is_loop_entry_point + } + + /// Get the description of a tail call type for logging + /// + /// # Arguments + /// * `is_recursive_call` - Whether this is a recursive call + /// * `is_loop_entry_point` - Whether this is the loop entry point + /// + /// # Returns + /// * A string describing the call type + pub fn call_type_description(is_recursive_call: bool, is_loop_entry_point: bool) -> &'static str { + if is_recursive_call { + "recursive" + } else if is_loop_entry_point { + "entry" + } else { + "normal" + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_is_recursive_call_true() { + assert!(TailCallDetectorBox::is_recursive_call("loop_step", "loop_step")); + } + + #[test] + fn test_is_recursive_call_false() { + assert!(!TailCallDetectorBox::is_recursive_call("loop_step", "other")); + assert!(!TailCallDetectorBox::is_recursive_call("main", "loop_step")); + } + + #[test] + fn test_is_loop_entry_call() { + assert!(TailCallDetectorBox::is_loop_entry_call(true)); + assert!(!TailCallDetectorBox::is_loop_entry_call(false)); + } + + #[test] + fn test_should_skip_param_binding_recursive() { + assert!(TailCallDetectorBox::should_skip_param_binding(true, false)); + } + + #[test] + fn test_should_skip_param_binding_entry() { + assert!(TailCallDetectorBox::should_skip_param_binding(false, true)); + } + + #[test] + fn test_should_skip_param_binding_normal() { + assert!(!TailCallDetectorBox::should_skip_param_binding(false, false)); + } + + #[test] + fn test_call_type_description() { + assert_eq!(TailCallDetectorBox::call_type_description(true, false), "recursive"); + assert_eq!(TailCallDetectorBox::call_type_description(false, true), "entry"); + assert_eq!(TailCallDetectorBox::call_type_description(false, false), "normal"); + } +}