diff --git a/src/mir/builder/control_flow/joinir/merge/mod.rs b/src/mir/builder/control_flow/joinir/merge/mod.rs index 715f9aff..94467c0e 100644 --- a/src/mir/builder/control_flow/joinir/merge/mod.rs +++ b/src/mir/builder/control_flow/joinir/merge/mod.rs @@ -23,12 +23,10 @@ mod tail_call_classifier; mod merge_result; // Phase 33-17: Re-export for use by other modules -pub use merge_result::MergeResult; -pub use tail_call_classifier::{TailCallKind, classify_tail_call}; -pub use loop_header_phi_info::{LoopHeaderPhiInfo, CarrierPhiEntry}; +pub use loop_header_phi_info::LoopHeaderPhiInfo; pub use loop_header_phi_builder::LoopHeaderPhiBuilder; -use crate::mir::{BasicBlockId, MirModule, ValueId}; +use crate::mir::{MirModule, ValueId}; use crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary; /// Phase 49-3.2: Merge JoinIR-generated MIR blocks into current_function @@ -157,30 +155,150 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( ); } + // Phase 33-20: Extract other carriers from exit_bindings + // Skip the loop variable (it's handled separately) and collect other carriers + let other_carriers: Vec<(String, ValueId)> = boundary.exit_bindings + .iter() + .filter(|b| b.carrier_name != *loop_var_name) + .map(|b| (b.carrier_name.clone(), b.host_slot)) + .collect(); + + if debug && !other_carriers.is_empty() { + eprintln!( + "[cf_loop/joinir] Phase 33-20: Found {} other carriers from exit_bindings: {:?}", + other_carriers.len(), + other_carriers.iter().map(|(n, _)| n.as_str()).collect::>() + ); + } + let phi_info = LoopHeaderPhiBuilder::build( builder, entry_block_remapped, // header_block (JoinIR's entry block = loop header) host_entry_block, // entry_block (host's block that jumps to loop header) loop_var_name, loop_var_init, - &[], // No other carriers for Pattern 2 (loop var is the only carrier) + &other_carriers, // Phase 33-20: Pass other carriers from exit_bindings boundary.expr_result.is_some(), // expr_result_is_loop_var debug, )?; - // Phase 33-16: Override remapper so that JoinIR's loop var parameter (ValueId(0)) - // references the PHI dst instead of a separate remapped value. - // This ensures all uses of the loop variable in the loop body use the PHI result. - if let Some(phi_dst) = phi_info.get_carrier_phi(loop_var_name) { - // JoinIR uses ValueId(0) for loop var in loop_step's first parameter - // After remapping, we want those uses to refer to the header PHI dst - remapper.set_value(ValueId(0), phi_dst); + // Phase 33-21: Override remapper for loop_step's parameters + // + // JoinIR generates separate parameter ValueIds for each function: + // - main(): ValueId(0), ValueId(1), ... for (i_init, carrier1_init, ...) + // - loop_step(): ValueId(3), ValueId(4), ... for (i_param, carrier1_param, ...) + // + // The loop body uses loop_step's parameters, so we need to remap THOSE + // to the header PHI dsts, not main()'s parameters. + // + // We get loop_step's parameters from function_params collected earlier. + // Phase 33-21: Override remapper for ALL functions' parameters + // + // JoinIR generates separate parameter ValueIds for each function: + // - main (join_func_0): ValueId(0), ValueId(1), ... for (i_init, carrier1_init, ...) + // - loop_step (join_func_1): ValueId(3), ValueId(4), ... for (i_param, carrier1_param, ...) + // + // ALL of these need to be mapped to header PHI dsts so that: + // 1. condition evaluation uses PHI result + // 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() + let main_func_name = "join_func_0"; + if function_params.get(main_func_name).is_none() { + eprintln!( + "[cf_loop/joinir] WARNING: function_params.get('{}') returned None. Available keys: {:?}", + main_func_name, + function_params.keys().collect::>() + ); + } + if let Some(main_params) = function_params.get(main_func_name) { if debug { eprintln!( - "[cf_loop/joinir] Phase 33-16: Override remap ValueId(0) → {:?} (PHI dst)", - phi_dst + "[cf_loop/joinir] Phase 33-21: main ({}) params: {:?}", + main_func_name, main_params ); } + // 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 { + eprintln!( + "[cf_loop/joinir] Phase 33-21: REMAP main param {:?} → {:?} ('{}')", + main_param, entry.phi_dst, carrier_name + ); + } + remapper.set_value(main_param, 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: {:?}", + function_params.keys().collect::>() + ); + } + if function_params.get(loop_step_func_name).is_none() { + eprintln!( + "[cf_loop/joinir] WARNING: function_params.get('{}') returned None. Available keys: {:?}", + loop_step_func_name, + function_params.keys().collect::>() + ); + } + if let Some(loop_step_params) = function_params.get(loop_step_func_name) { + if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-21: loop_step ({}) params: {:?}", + loop_step_func_name, loop_step_params + ); + } + // Map loop_step's parameters to header PHI dsts + // loop_step params: [i_param, carrier1_param, ...] + // 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) { + if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-21: REMAP loop_step param {:?} → {:?} ('{}')", + loop_step_param, entry.phi_dst, carrier_name + ); + } + remapper.set_value(loop_step_param, entry.phi_dst); + } + } + } + + if function_params.get(main_func_name).is_none() && function_params.get(loop_step_func_name).is_none() { + // 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 { + eprintln!( + "[cf_loop/joinir] Phase 33-16 fallback: Override remap ValueId(0) → {:?} (PHI dst)", + phi_dst + ); + } + } + for (idx, (carrier_name, entry)) in phi_info.carrier_phis.iter().enumerate() { + if carrier_name == loop_var_name { + continue; + } + let join_value_id = ValueId(idx as u32); + 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 + ); + } + } } phi_info @@ -222,9 +340,9 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( )?; } - // Phase 5: Build exit PHI (expr result and carrier PHIs) - // Phase 33-13: Now also builds carrier PHIs and returns their mapping - let (exit_phi_result_id, carrier_phis) = exit_phi_builder::build_exit_phi( + // Phase 5: Build exit PHI (expr result only, not carrier PHIs) + // Phase 33-20: Carrier PHIs are now taken from header PHI info, not exit block + let (exit_phi_result_id, _exit_carrier_phis) = exit_phi_builder::build_exit_phi( builder, merge_result.exit_block_id, &merge_result.exit_phi_inputs, @@ -232,10 +350,26 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( debug, )?; + // Phase 33-20: Build carrier_phis from header PHI info instead of exit PHI builder + // The header PHI dst IS the current value of each carrier, and it's SSA-valid + // because it's defined at the loop header and dominates the exit block. + let carrier_phis: std::collections::BTreeMap = loop_header_phi_info + .carrier_phis + .iter() + .map(|(name, entry)| (name.clone(), entry.phi_dst)) + .collect(); + + if debug && !carrier_phis.is_empty() { + eprintln!( + "[cf_loop/joinir] Phase 33-20: Using header PHI dsts for variable_map: {:?}", + carrier_phis.iter().map(|(n, v)| (n.as_str(), v)).collect::>() + ); + } + // Phase 6: Reconnect boundary (if specified) // Phase 197-B: Pass remapper to enable per-carrier exit value lookup // Phase 33-10-Refactor-P3: Delegate to ExitLineOrchestrator - // Phase 33-13: Pass carrier_phis for proper variable_map update + // Phase 33-20: Now uses header PHI dsts instead of exit PHI dsts if let Some(boundary) = boundary { exit_line::ExitLineOrchestrator::execute(builder, boundary, &carrier_phis, debug)?; } diff --git a/src/mir/builder/control_flow/joinir/patterns/pattern4_with_continue.rs b/src/mir/builder/control_flow/joinir/patterns/pattern4_with_continue.rs index 225da303..9caf356e 100644 --- a/src/mir/builder/control_flow/joinir/patterns/pattern4_with_continue.rs +++ b/src/mir/builder/control_flow/joinir/patterns/pattern4_with_continue.rs @@ -1,54 +1,35 @@ -//! Phase 195: Pattern 4 (Loop with Continue) - STUB IMPLEMENTATION +//! Phase 33-21: Pattern 4 (Loop with Continue) Implementation //! -//! **Current Status**: Not implemented. Returns explicit error. +//! **Current Status**: Fully implemented using Select-based continue semantics. //! -//! ## Why Deferred to Phase 195+? +//! ## Implementation Approach //! -//! Continue semantics are complex compared to break: -//! - Break: Jump directly to exit (simple control flow) -//! - Continue: Jump to loop latch, then to loop condition (requires latch block) -//! - Requires additional PHI merging at latch block -//! - Interacts with other loop transformations +//! Continue is handled via Select instruction for carrier updates: +//! - When continue condition is true: carrier keeps its value (no update) +//! - When continue condition is false: carrier gets updated value //! -//! ## Migration Path (Use These Instead) +//! This avoids the complexity of explicit latch blocks by using PHI-based +//! value selection at each iteration. +//! +//! ## Example //! -//! ### Pattern 1: Simple While (No break/continue) -//! ```nyash -//! loop(i < 10) { i = i + 1 } -//! ``` -//! -//! ### Pattern 2: Loops with Break //! ```nyash +//! local i = 0 +//! local sum = 0 //! loop(i < 10) { -//! if i >= 5 { break } //! i = i + 1 +//! if i % 2 == 0 { continue } // Skip even numbers +//! sum = sum + i //! } +//! // sum = 25 (1+3+5+7+9) //! ``` //! -//! ### Pattern 3: Loops with If + PHI (Workaround for continue) -//! ```nyash -//! loop(i < 10) { -//! if i % 2 == 0 { -//! // Process even -//! x = process(i) -//! } else { -//! // Skip odd (simulates continue) -//! x = default_value -//! } -//! i = i + 1 -//! } -//! ``` +//! ## Key Implementation Details //! -//! ## Design Philosophy -//! Rather than silently fail or return incorrect MIR, -//! Pattern 4 returns **explicit error** directing users to working patterns. -//! This is better than: -//! - Silent failure (confusing bugs) -//! - Incorrect lowering (wrong semantics) -//! - Feature gates (hidden complexity) -//! -//! ## Implementation Plan (Phase 195+) -//! See: docs/development/proposals/phase-195-pattern4.md +//! - **Select instruction**: Used to conditionally skip carrier updates +//! - **Header PHI**: Tracks carrier values across iterations +//! - **ExitMeta**: Maps final carrier values to host variable slots +//! - **Phase 33-21 fix**: Correct remapping of function parameters to header PHI dsts use crate::ast::ASTNode; use crate::mir::builder::MirBuilder; @@ -81,39 +62,35 @@ pub fn can_lower(_builder: &MirBuilder, ctx: &super::router::LoopPatternContext) ctx.pattern_kind == LoopPatternKind::Pattern4Continue } -/// Phase 194+: Lowering function for Pattern 4 -/// -/// **STUB IMPLEMENTATION**: Returns explicit error. Pattern 4 is not yet implemented. +/// Phase 33-19: Lowering function for Pattern 4 /// /// Wrapper around cf_loop_pattern4_with_continue to match router signature. /// -/// # Future Implementation Plan +/// # Implementation (Phase 195-197) /// -/// When implemented, this will: /// 1. Extract loop variables from condition -/// 2. Generate JoinIR with continue support +/// 2. Generate JoinIR with continue support (lower_loop_with_continue_minimal) /// 3. Convert JoinModule → MirModule /// 4. Create JoinInlineBoundary for input/output mapping /// 5. Merge MIR blocks into current_function -/// 6. Return Void (loop doesn't produce values) +/// 6. Return loop result (first carrier value) pub fn lower( - _builder: &mut MirBuilder, + builder: &mut MirBuilder, ctx: &super::router::LoopPatternContext, ) -> Result, String> { - Err(format!( - "[cf_loop/pattern4] Pattern 4 (continue) not yet implemented for '{}' loop.\n\ - Use Pattern 1-3 instead (simple while, break, or if+phi patterns).\n\ - See: docs/development/proposals/phase-195-pattern4.md", - ctx.func_name - )) + // Phase 33-19: Connect stub to actual implementation + builder.cf_loop_pattern4_with_continue( + ctx.condition, + ctx.body, + ctx.func_name, + ctx.debug, + ) } impl MirBuilder { - /// Phase 194+: Pattern 4 (Loop with Continue) minimal lowerer + /// Phase 33-21: Pattern 4 (Loop with Continue) lowerer /// - /// **STUB IMPLEMENTATION**: This function is a placeholder for future implementation. - /// - /// Handles loops with continue statements that skip to next iteration. + /// Handles loops with continue statements using Select-based carrier updates. /// /// # Example /// @@ -130,26 +107,15 @@ impl MirBuilder { /// // sum = 25 (1+3+5+7+9) /// ``` /// - /// # Implementation Status + /// # Implementation /// - /// **NOT IMPLEMENTED**: This is a stub. Pattern 4 lowering logic needs to be implemented. - /// - /// The lowerer should: - /// 1. Detect continue statements in the loop body - /// 2. Generate appropriate PHI nodes for continue targets - /// 3. Handle carrier variables (i, sum) across continue boundaries - /// 4. Generate exit PHI nodes for final values - /// - /// # Steps (TODO) - /// - /// 1. Extract loop variables (i, sum) - /// 2. Generate JoinIR using loop_with_continue_minimal (not yet implemented) - /// 3. Convert JoinModule → MirModule - /// 4. Create JoinInlineBoundary for input/output mapping - /// 5. Merge MIR blocks into current_function - /// 6. Return Void (loop doesn't produce values) - #[doc(hidden)] // Not ready for public use - #[allow(dead_code)] // Will be implemented in Phase 195+ + /// 1. Extract loop variable from condition + /// 2. Build CarrierInfo from variable_map + /// 3. Analyze carrier update expressions from loop body + /// 4. Generate JoinIR with Select-based continue semantics + /// 5. Convert JoinModule → MirModule + /// 6. Merge MIR blocks with header PHI and exit bindings + /// 7. Return Void (loop result accessed via variable_map) pub(in crate::mir::builder) fn cf_loop_pattern4_with_continue( &mut self, condition: &ASTNode, @@ -339,17 +305,23 @@ impl MirBuilder { &format!("join_inputs: {:?}", join_inputs.iter().map(|v| v.0).collect::>()) ); - let boundary = crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary::new_with_exit_bindings( + let mut boundary = crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary::new_with_exit_bindings( join_inputs, // JoinIR's main() parameters (dynamic) host_inputs, // Host's loop variables (dynamic) exit_bindings, ); + // Phase 33-19: Set loop_var_name for proper exit PHI collection + boundary.loop_var_name = Some(loop_var_name.clone()); + // Phase 195: Capture exit PHI result (Pattern 4 returns sum) - let result_val = self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?; + let _result_val = self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?; + + // Phase 33-19: Like Pattern3, return void (loop result is read via variable_map) + let void_val = crate::mir::builder::emission::constant::emit_void(self); // Phase 195: Use unified trace - trace::trace().debug("pattern4", &format!("Loop complete, returning result {:?}", result_val)); + trace::trace().debug("pattern4", &format!("Loop complete, returning Void {:?}", void_val)); - Ok(result_val) + Ok(Some(void_val)) } } diff --git a/src/mir/join_ir/lowering/loop_with_break_minimal.rs b/src/mir/join_ir/lowering/loop_with_break_minimal.rs index bc214c15..357d8897 100644 --- a/src/mir/join_ir/lowering/loop_with_break_minimal.rs +++ b/src/mir/join_ir/lowering/loop_with_break_minimal.rs @@ -60,7 +60,7 @@ use crate::mir::join_ir::lowering::carrier_info::{ExitMeta, JoinFragmentMeta}; use crate::mir::join_ir::lowering::condition_to_joinir::{lower_condition_to_joinir, ConditionEnv}; use crate::mir::join_ir::lowering::loop_scope_shape::LoopScopeShape; use crate::mir::join_ir::{ - BinOpKind, CompareOp, ConstValue, JoinFuncId, JoinFunction, JoinInst, JoinModule, + BinOpKind, ConstValue, JoinFuncId, JoinFunction, JoinInst, JoinModule, MirLikeInst, UnaryOp, }; use crate::mir::ValueId;