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 37bec1bf..f5d8c81b 100644 --- a/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs +++ b/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs @@ -8,8 +8,66 @@ use crate::mir::{BasicBlock, BasicBlockId, MirInstruction, MirModule, ValueId}; use crate::mir::builder::joinir_id_remapper::JoinIrIdRemapper; use crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary; +use super::loop_header_phi_builder::LoopHeaderPhiInfo; use std::collections::HashMap; +/// Phase 33-16: Tail Call Classification +/// +/// Classifies tail calls in JoinIR loops into three semantic categories: +/// +/// 1. **LoopEntry**: First entry into the loop (main → loop_step) +/// - Occurs from the entry function's entry block +/// - Should jump directly to target (not redirect to header) +/// - Reason: The entry block IS the header block; redirecting creates self-loop +/// +/// 2. **BackEdge**: Loop continuation (loop_step → loop_step) +/// - Occurs from loop body blocks (not entry function's entry block) +/// - MUST redirect to header block where PHI nodes are located +/// - Reason: PHI nodes need to merge values from all back edges +/// +/// 3. **ExitJump**: Loop termination (→ k_exit) +/// - Occurs when jumping to continuation functions +/// - Handled separately via Return conversion +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum TailCallKind { + /// First entry into loop - no redirection needed + LoopEntry, + /// Back edge in loop - redirect to header PHI + BackEdge, + /// Exit from loop - becomes Return conversion + ExitJump, +} + +/// Classifies a tail call based on context +/// +/// # Arguments +/// * `is_entry_func_entry_block` - True if this is the first function's first block (loop entry point) +/// * `has_loop_header_phis` - True if loop header PHI nodes exist +/// * `has_boundary` - True if JoinInlineBoundary exists (indicates loop context) +/// +/// # Returns +/// The classification of this tail call +fn classify_tail_call( + is_entry_func_entry_block: bool, + has_loop_header_phis: bool, + has_boundary: bool, +) -> TailCallKind { + // Entry function's entry block is the loop entry point + // It already IS at the header, so no redirection needed + if is_entry_func_entry_block { + return TailCallKind::LoopEntry; + } + + // If we have boundary and header PHIs, this is a back edge + // Must redirect to header for PHI merging + if has_boundary && has_loop_header_phis { + return TailCallKind::BackEdge; + } + + // Otherwise, treat as exit (will be handled by Return conversion) + TailCallKind::ExitJump +} + /// Phase 33-13: Return type for merge_and_rewrite /// /// Contains all information needed for exit PHI construction. @@ -26,6 +84,12 @@ pub struct MergeResult { /// /// Returns: /// - MergeResult containing exit_block_id, exit_phi_inputs, and carrier_inputs +/// +/// # Phase 33-16 +/// +/// The `loop_header_phi_info` parameter is used to: +/// 1. Set latch_incoming when processing tail calls +/// 2. Provide PHI dsts for exit value collection (instead of undefined parameters) pub(super) fn merge_and_rewrite( builder: &mut crate::mir::builder::MirBuilder, mir_module: &MirModule, @@ -33,6 +97,7 @@ pub(super) fn merge_and_rewrite( value_to_func_name: &HashMap, function_params: &HashMap>, boundary: Option<&JoinInlineBoundary>, + loop_header_phi_info: &mut LoopHeaderPhiInfo, debug: bool, ) -> Result { // Create exit block for Return conversion (single for all functions) @@ -77,15 +142,36 @@ pub(super) fn merge_and_rewrite( let entry_func_name = functions_merge.first().map(|(name, _)| name.as_str()); for (func_name, func) in functions_merge { + // Phase 33-15: Identify continuation functions (join_func_2 = k_exit, etc.) + // Continuation functions receive values from Jump args, not as independent sources + // We should NOT collect their Return values for exit_phi_inputs + // Note: MIR uses "join_func_N" naming, where N=2 is typically k_exit + let is_continuation_func = func_name == "join_func_2" || func_name.ends_with("k_exit"); + if debug { eprintln!( - "[cf_loop/joinir] Merging function '{}' with {} blocks, entry={:?}", + "[cf_loop/joinir] Merging function '{}' with {} blocks, entry={:?} (is_continuation={})", func_name, func.blocks.len(), - func.entry_block + func.entry_block, + is_continuation_func ); } + // Phase 33-15: Skip continuation function blocks entirely + // Continuation functions (k_exit) are just exit points; their parameters are + // passed via Jump args which become Return values in other functions. + // Processing continuation functions would add undefined ValueIds to PHI. + if is_continuation_func { + if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-15: Skipping continuation function '{}' blocks", + func_name + ); + } + continue; + } + // Build a local block map for this function (for remap_instruction compatibility) let mut local_block_map: HashMap = HashMap::new(); for old_block_id in func.blocks.keys() { @@ -115,8 +201,12 @@ pub(super) fn merge_and_rewrite( BasicBlock::new(new_block_id) }; - // Phase 189 FIX: Check if this is entry function's entry block (for boundary input skipping) - let is_entry_func_entry_block = + // Phase 33-16: Identify loop entry point + // + // The entry function's entry block is special: it IS the loop header. + // Redirecting tail calls from this block to itself would create a self-loop. + // This flag is used to prevent such incorrect redirection. + let is_loop_entry_point = entry_func_name == Some(func_name.as_str()) && *old_block_id == func.entry_block; // DEBUG: Print block being processed @@ -159,7 +249,7 @@ pub(super) fn merge_and_rewrite( } // Phase 189 FIX: Skip Const instructions in entry function's entry block // that initialize boundary inputs. BoundaryInjector provides these values via Copy. - if is_entry_func_entry_block && boundary_input_set.contains(dst) { + if is_loop_entry_point && boundary_input_set.contains(dst) { if debug { eprintln!("[cf_loop/joinir] Skipping boundary input const (replaced by BoundaryInjector Copy): {:?}", inst); } @@ -296,9 +386,74 @@ pub(super) fn merge_and_rewrite( } } - // Set terminator to Jump + // Phase 33-16: Record latch incoming for loop header PHI + // + // At this point, args[0] contains the updated loop variable value (i_next). + // We record this as the latch incoming so that the header PHI can reference + // the correct SSA value at loop continuation time. + if let Some(b) = boundary { + if let Some(loop_var_name) = &b.loop_var_name { + if !args.is_empty() { + // The first arg is the loop variable's updated value + let latch_value = args[0]; + // The current block (new_block_id) is the latch block + loop_header_phi_info.set_latch_incoming( + loop_var_name, + new_block_id, // latch block + latch_value, // updated loop var value (i_next) + ); + + if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-16: Set latch incoming for '{}': block={:?}, value={:?}", + loop_var_name, new_block_id, latch_value + ); + } + } + } + } + + // Phase 33-16: Classify tail call to determine redirection behavior + // + // Tail calls have different semantics depending on where they occur: + // - LoopEntry: First entry (entry function's entry block) → no redirect + // - BackEdge: Loop continuation (other blocks) → redirect to header PHI + // - ExitJump: Exit to continuation → handled by Return conversion + let tail_call_kind = classify_tail_call( + is_loop_entry_point, + !loop_header_phi_info.carrier_phis.is_empty(), + boundary.is_some(), + ); + + let actual_target = match tail_call_kind { + TailCallKind::BackEdge => { + // Back edge: redirect to header block where PHIs are + if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-16: BackEdge detected, redirecting from {:?} to header {:?}", + target_block, loop_header_phi_info.header_block + ); + } + loop_header_phi_info.header_block + } + TailCallKind::LoopEntry => { + // Loop entry: no redirect (entry block IS the header) + if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-16: LoopEntry detected, using direct target {:?}", + target_block + ); + } + target_block + } + TailCallKind::ExitJump => { + // Exit: use target as-is (will be handled by Return conversion) + target_block + } + }; + new_block.terminator = Some(MirInstruction::Jump { - target: target_block, + target: actual_target, }); // DEBUG: Print final state after adding parameter bindings @@ -329,42 +484,53 @@ pub(super) fn merge_and_rewrite( MirInstruction::Return { value } => { // Convert Return to Jump to exit block // All functions return to same exit block (Phase 189) - // Phase 189-Fix: Add Copy instruction to pass return value to exit PHI - // Phase 33-14: Only add to exit_phi_inputs if boundary.expr_result is Some - if let Some(ret_val) = value { - let remapped_val = remapper.get_value(*ret_val).unwrap_or(*ret_val); - if debug { - eprintln!( - "[cf_loop/joinir] Return({:?}) → Jump to exit", - remapped_val - ); - } - // Phase 33-14: Only collect for exit PHI if this loop has an expr result - // This separates "loop as expression" from "carrier-only loop" - let has_expr_result = boundary.map(|b| b.expr_result.is_some()).unwrap_or(true); - if has_expr_result { - // Collect (from_block, return_value) for exit PHI generation - exit_phi_inputs.push((new_block_id, remapped_val)); - } else if debug { - eprintln!( - "[cf_loop/joinir] Phase 33-14: Skipping exit_phi_inputs (carrier-only pattern)" - ); + // + // Phase 33-16: Use loop header PHI dst for exit values + // + // Instead of referencing undefined parameters, we use the header PHI dst + // which is SSA-defined and tracks the current loop iteration value. + // + if let Some(_ret_val) = value { + // Phase 33-16: Collect exit_phi_inputs using header PHI dst + // + // If we have a loop_var_name and corresponding header PHI, + // use the PHI dst instead of the remapped parameter value. + // This is the key fix for the SSA-undef problem! + if let Some(b) = boundary { + if let Some(loop_var_name) = &b.loop_var_name { + if let Some(phi_dst) = loop_header_phi_info.get_carrier_phi(loop_var_name) { + // Use PHI dst (SSA-defined!) + exit_phi_inputs.push((new_block_id, phi_dst)); + if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-16: Using header PHI dst {:?} for exit (loop_var='{}')", + phi_dst, loop_var_name + ); + } + } else if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-16: No header PHI found for '{}', skipping exit_phi_inputs", + loop_var_name + ); + } + } } } - // Phase 33-13: Collect carrier exit values for carrier PHIs - // When jumping to exit_block, collect each carrier's exit value + // Phase 33-16: Collect carrier exit values using header PHI dsts + // + // For each carrier, use the header PHI dst instead of + // the undefined exit binding value. if let Some(b) = boundary { for binding in &b.exit_bindings { - if let Some(remapped_exit) = remapper.get_value(binding.join_exit_value) { - carrier_inputs - .entry(binding.carrier_name.clone()) + 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, remapped_exit)); + .push((new_block_id, phi_dst)); if debug { eprintln!( - "[cf_loop/joinir] Phase 33-13: Carrier '{}' exit: ({:?}, {:?})", - binding.carrier_name, new_block_id, remapped_exit + "[cf_loop/joinir] Phase 33-16: Using header PHI dst {:?} for carrier '{}'", + phi_dst, binding.carrier_name ); } } diff --git a/src/mir/builder/control_flow/joinir/merge/loop_header_phi_builder.rs b/src/mir/builder/control_flow/joinir/merge/loop_header_phi_builder.rs new file mode 100644 index 00000000..9581ff86 --- /dev/null +++ b/src/mir/builder/control_flow/joinir/merge/loop_header_phi_builder.rs @@ -0,0 +1,318 @@ +//! Loop Header PHI Builder +//! +//! Phase 33-16: Generates PHI nodes at loop header blocks to track carrier values. +//! +//! ## Problem +//! +//! JoinIR uses function parameters (i_param, i_exit) to pass values between +//! iterations. When inlined into MIR, these parameters have no SSA definition. +//! This causes SSA-undef errors when exit PHIs reference remapped parameters. +//! +//! ## Solution +//! +//! Generate a PHI node at the loop header block for each carrier variable: +//! +//! ```text +//! loop_header: +//! i_phi = PHI [(entry_block, i_init), (latch_block, i_next)] +//! // rest of loop... +//! ``` +//! +//! The PHI's dst becomes the "current value" of the carrier during iteration. +//! Exit paths should reference this PHI dst, not the undefined parameters. +//! +//! ## Usage +//! +//! Called from merge pipeline between Phase 3 (remap_values) and Phase 4 +//! (instruction_rewriter). + +use crate::mir::{BasicBlock, BasicBlockId, MirInstruction, ValueId}; +use std::collections::BTreeMap; + +/// Information about loop header PHIs +/// +/// Generated by LoopHeaderPhiBuilder and used by: +/// - exit_phi_builder: to reference the current loop value +/// - ExitLineReconnector: to update variable_map with final values +#[derive(Debug, Clone)] +pub struct LoopHeaderPhiInfo { + /// The block where header PHIs are placed + pub header_block: BasicBlockId, + + /// Carrier variable PHI mappings: carrier_name → PHI dst + /// + /// The PHI dst is the ValueId that represents the "current value" + /// of this carrier during loop iteration. + pub carrier_phis: BTreeMap, + + /// Expression result PHI dst (if loop is used as expression) + /// + /// For Pattern 2 (joinir_min_loop), this is the same as the loop + /// variable's PHI dst. For carrier-only patterns (trim), this is None. + pub expr_result_phi: Option, +} + +/// Entry for a single carrier's header PHI +#[derive(Debug, Clone)] +pub struct CarrierPhiEntry { + /// PHI destination ValueId (the "current value" during iteration) + pub phi_dst: ValueId, + + /// Entry edge: (from_block, init_value) + pub entry_incoming: (BasicBlockId, ValueId), + + /// Latch edge: (from_block, updated_value) - set after instruction rewrite + pub latch_incoming: Option<(BasicBlockId, ValueId)>, +} + +impl LoopHeaderPhiInfo { + /// Create empty LoopHeaderPhiInfo + pub fn empty(header_block: BasicBlockId) -> Self { + Self { + header_block, + carrier_phis: BTreeMap::new(), + expr_result_phi: None, + } + } + + /// Get the PHI dst for a carrier variable + pub fn get_carrier_phi(&self, name: &str) -> Option { + self.carrier_phis.get(name).map(|e| e.phi_dst) + } + + /// Set latch incoming for a carrier + /// + /// Called from instruction_rewriter after processing tail call Copy instructions. + pub fn set_latch_incoming(&mut self, name: &str, from_block: BasicBlockId, value: ValueId) { + if let Some(entry) = self.carrier_phis.get_mut(name) { + entry.latch_incoming = Some((from_block, value)); + } + } + + /// Check if all carriers have latch incoming set + pub fn all_latch_set(&self) -> bool { + self.carrier_phis.values().all(|e| e.latch_incoming.is_some()) + } +} + +/// Builder for loop header PHIs +/// +/// Generates PHI nodes at the loop header block to track carrier values. +pub struct LoopHeaderPhiBuilder; + +impl LoopHeaderPhiBuilder { + /// Generate header PHIs for loop carriers + /// + /// # Arguments + /// + /// * `builder` - MirBuilder for allocating ValueIds + /// * `header_block` - The loop header block ID + /// * `entry_block` - The block that jumps to header on first iteration + /// * `carrier_info` - Carrier variable metadata from pattern lowerer + /// * `remapper` - ID remapper for looking up remapped init values + /// * `debug` - Enable debug output + /// + /// # Returns + /// + /// LoopHeaderPhiInfo with allocated PHI dsts. + /// Note: latch_incoming is not yet set - that happens in instruction_rewriter. + pub fn build( + builder: &mut crate::mir::builder::MirBuilder, + header_block: BasicBlockId, + entry_block: BasicBlockId, + loop_var_name: &str, + loop_var_init: ValueId, + carriers: &[(String, ValueId)], // (name, init_value) pairs + expr_result_is_loop_var: bool, + debug: bool, + ) -> Result { + if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-16: Building header PHIs at {:?}", + header_block + ); + eprintln!( + "[cf_loop/joinir] Loop var '{}' init={:?}, entry_block={:?}", + loop_var_name, loop_var_init, entry_block + ); + } + + let mut info = LoopHeaderPhiInfo::empty(header_block); + + // Allocate PHI for loop variable + let loop_var_phi_dst = builder.next_value_id(); + info.carrier_phis.insert( + loop_var_name.to_string(), + CarrierPhiEntry { + phi_dst: loop_var_phi_dst, + entry_incoming: (entry_block, loop_var_init), + latch_incoming: None, + }, + ); + + if debug { + eprintln!( + "[cf_loop/joinir] Loop var PHI: {:?} = phi [(from {:?}, {:?}), (latch TBD)]", + loop_var_phi_dst, entry_block, loop_var_init + ); + } + + // Allocate PHIs for other carriers + for (name, init_value) in carriers { + let phi_dst = builder.next_value_id(); + info.carrier_phis.insert( + name.clone(), + CarrierPhiEntry { + phi_dst, + entry_incoming: (entry_block, *init_value), + latch_incoming: None, + }, + ); + + if debug { + eprintln!( + "[cf_loop/joinir] Carrier '{}' PHI: {:?} = phi [(from {:?}, {:?}), (latch TBD)]", + name, phi_dst, entry_block, init_value + ); + } + } + + // Set expr_result if this pattern returns the loop var + if expr_result_is_loop_var { + info.expr_result_phi = Some(loop_var_phi_dst); + if debug { + eprintln!( + "[cf_loop/joinir] expr_result = {:?} (loop var PHI)", + loop_var_phi_dst + ); + } + } + + Ok(info) + } + + /// Finalize header PHIs by inserting them into the MIR block + /// + /// Called after instruction_rewriter has set all latch_incoming values. + /// + /// # Arguments + /// + /// * `builder` - MirBuilder containing the current function + /// * `info` - LoopHeaderPhiInfo with all incoming edges set + /// * `debug` - Enable debug output + pub fn finalize( + builder: &mut crate::mir::builder::MirBuilder, + info: &LoopHeaderPhiInfo, + debug: bool, + ) -> Result<(), String> { + if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-16: Finalizing header PHIs at {:?}", + info.header_block + ); + } + + // Validate all latch incoming are set + for (name, entry) in &info.carrier_phis { + if entry.latch_incoming.is_none() { + return Err(format!( + "Phase 33-16: Carrier '{}' has no latch incoming set", + name + )); + } + } + + // Get the header block from current function + let current_func = builder.current_function.as_mut().ok_or( + "Phase 33-16: No current function when finalizing header PHIs" + )?; + + let header_block = current_func.blocks.get_mut(&info.header_block).ok_or_else(|| { + format!( + "Phase 33-16: Header block {:?} not found in current function", + info.header_block + ) + })?; + + // Insert PHIs at the beginning of the header block (before other instructions) + // Sorted by carrier name for determinism + let mut phi_instructions: Vec = Vec::new(); + + for (name, entry) in &info.carrier_phis { + let (entry_block, entry_val) = entry.entry_incoming; + let (latch_block, latch_val) = entry.latch_incoming.unwrap(); + + let phi = MirInstruction::Phi { + dst: entry.phi_dst, + inputs: vec![(entry_block, entry_val), (latch_block, latch_val)], + type_hint: None, + }; + + phi_instructions.push(phi); + + if debug { + eprintln!( + "[cf_loop/joinir] Finalized carrier '{}' PHI: {:?} = phi [({:?}, {:?}), ({:?}, {:?})]", + name, entry.phi_dst, entry_block, entry_val, latch_block, latch_val + ); + } + } + + // Prepend PHIs to existing instructions + let mut new_instructions = phi_instructions; + new_instructions.append(&mut header_block.instructions); + header_block.instructions = new_instructions; + + // Also prepend spans for the PHIs + let mut new_spans: Vec = (0..info.carrier_phis.len()) + .map(|_| crate::ast::Span::unknown()) + .collect(); + new_spans.append(&mut header_block.instruction_spans); + header_block.instruction_spans = new_spans; + + if debug { + eprintln!( + "[cf_loop/joinir] Header block now has {} instructions", + header_block.instructions.len() + ); + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_loop_header_phi_info_creation() { + let header = BasicBlockId(10); + let info = LoopHeaderPhiInfo::empty(header); + + assert_eq!(info.header_block, header); + assert!(info.carrier_phis.is_empty()); + assert!(info.expr_result_phi.is_none()); + } + + #[test] + fn test_carrier_phi_entry() { + let mut info = LoopHeaderPhiInfo::empty(BasicBlockId(10)); + + info.carrier_phis.insert( + "i".to_string(), + CarrierPhiEntry { + phi_dst: ValueId(100), + entry_incoming: (BasicBlockId(1), ValueId(5)), + latch_incoming: None, + }, + ); + + assert_eq!(info.get_carrier_phi("i"), Some(ValueId(100))); + assert_eq!(info.get_carrier_phi("x"), None); + assert!(!info.all_latch_set()); + + info.set_latch_incoming("i", BasicBlockId(20), ValueId(50)); + assert!(info.all_latch_set()); + } +} diff --git a/src/mir/builder/control_flow/joinir/merge/mod.rs b/src/mir/builder/control_flow/joinir/merge/mod.rs index e937b577..501f52e9 100644 --- a/src/mir/builder/control_flow/joinir/merge/mod.rs +++ b/src/mir/builder/control_flow/joinir/merge/mod.rs @@ -17,8 +17,9 @@ mod value_collector; mod instruction_rewriter; mod exit_phi_builder; pub mod exit_line; +pub mod loop_header_phi_builder; -use crate::mir::{MirModule, ValueId}; +use crate::mir::{BasicBlockId, MirModule, ValueId}; use crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary; /// Phase 49-3.2: Merge JoinIR-generated MIR blocks into current_function @@ -105,7 +106,84 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( // Phase 3: Remap ValueIds remap_values(builder, &used_values, &mut remapper, debug)?; + // 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: + // 1. Tail call handling can set latch_incoming + // 2. Return handling can use PHI dsts for exit values + let (entry_func_name, entry_func) = mir_module + .functions + .iter() + .next() + .ok_or("JoinIR module has no functions (Phase 3.5)")?; + let entry_block_remapped = remapper + .get_block(entry_func_name, entry_func.entry_block) + .ok_or_else(|| format!("Entry block not found for {} (Phase 3.5)", entry_func_name))?; + + // Phase 33-16: Get host's current block as the entry edge (the block that jumps INTO the loop) + let host_entry_block = builder.current_block.ok_or( + "Phase 33-16: No current block when building header PHIs" + )?; + + let mut loop_header_phi_info = if let Some(boundary) = boundary { + if let Some(loop_var_name) = &boundary.loop_var_name { + // Phase 33-16: Get loop variable's initial value from HOST (not JoinIR's ValueId(0)) + // boundary.host_inputs[0] is the host ValueId that holds the initial loop var value + let loop_var_init = boundary.host_inputs.first().copied().ok_or( + "Phase 33-16: No host_inputs in boundary for loop_var_init" + )?; + + if debug { + eprintln!( + "[cf_loop/joinir] Phase 3.5: Building header PHIs for loop_var='{}' at {:?}", + loop_var_name, entry_block_remapped + ); + eprintln!( + "[cf_loop/joinir] loop_var_init={:?} (from boundary.host_inputs[0])", + loop_var_init + ); + eprintln!( + "[cf_loop/joinir] host_entry_block={:?} (where initial value comes from)", + host_entry_block + ); + } + + let phi_info = loop_header_phi_builder::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) + 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); + if debug { + eprintln!( + "[cf_loop/joinir] Phase 33-16: Override remap ValueId(0) → {:?} (PHI dst)", + phi_dst + ); + } + } + + phi_info + } else { + loop_header_phi_builder::LoopHeaderPhiInfo::empty(entry_block_remapped) + } + } else { + loop_header_phi_builder::LoopHeaderPhiInfo::empty(entry_block_remapped) + }; + // Phase 4: Merge blocks and rewrite instructions + // Phase 33-16: Pass mutable loop_header_phi_info for latch_incoming tracking let merge_result = instruction_rewriter::merge_and_rewrite( builder, mir_module, @@ -113,9 +191,28 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( &value_to_func_name, &function_params, boundary, + &mut loop_header_phi_info, debug, )?; + // Phase 4.5: Finalize loop header PHIs (insert into header block) + // + // By now, instruction_rewriter has set latch_incoming for all carriers. + // We can finalize the PHIs and insert them into the header block. + if !loop_header_phi_info.carrier_phis.is_empty() { + if debug { + eprintln!( + "[cf_loop/joinir] Phase 4.5: Finalizing {} header PHIs", + loop_header_phi_info.carrier_phis.len() + ); + } + loop_header_phi_builder::LoopHeaderPhiBuilder::finalize( + builder, + &loop_header_phi_info, + debug, + )?; + } + // 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( @@ -137,14 +234,8 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( let exit_block_id = merge_result.exit_block_id; // Jump from current block to entry function's entry block - let (entry_func_name, entry_func) = mir_module - .functions - .iter() - .next() - .ok_or("JoinIR module has no functions")?; - let entry_block = remapper - .get_block(entry_func_name, entry_func.entry_block) - .ok_or_else(|| format!("Entry block not found for {}", entry_func_name))?; + // (Reuse entry_func_name and entry_block_remapped from Phase 3.5) + let entry_block = entry_block_remapped; if debug { eprintln!("[cf_loop/joinir] Entry function name: {}", entry_func_name); diff --git a/src/mir/builder/control_flow/joinir/merge/value_collector.rs b/src/mir/builder/control_flow/joinir/merge/value_collector.rs index 37dde612..28548114 100644 --- a/src/mir/builder/control_flow/joinir/merge/value_collector.rs +++ b/src/mir/builder/control_flow/joinir/merge/value_collector.rs @@ -73,10 +73,24 @@ pub(super) fn collect_values( } } - // Also collect parameter ValueIds - for param in &func.params { - used_values.insert(*param); - } + // Phase 33-15: DO NOT collect parameter ValueIds into used_values + // + // Reasoning: Parameters are implicitly defined at function entry in JoinIR. + // When inlined into host MIR, if parameters are remapped, the remapped ValueIds + // have no SSA definition (causing SSA-undef errors). + // + // Instead, parameters should remain with their original JoinIR ValueIds. + // These are properly defined via: + // 1. BoundaryInjector Copy (for entry function: host_input → join_param) + // 2. Tail call Copy (for recursive calls: call_arg → param) + // + // We still store parameters in function_params for tail call handling, + // but don't add them to used_values so they won't be remapped. + // + // OLD CODE (removed): + // for param in &func.params { + // used_values.insert(*param); + // } } if debug { diff --git a/src/mir/builder/control_flow/joinir/patterns/exit_binding.rs b/src/mir/builder/control_flow/joinir/patterns/exit_binding.rs index 221db5b5..ac0437be 100644 --- a/src/mir/builder/control_flow/joinir/patterns/exit_binding.rs +++ b/src/mir/builder/control_flow/joinir/patterns/exit_binding.rs @@ -389,6 +389,7 @@ mod tests { condition_inputs: vec![], // Phase 171: Add missing field condition_bindings: vec![], // Phase 171-fix: Add missing field expr_result: None, // Phase 33-14: Add missing field + loop_var_name: None, // Phase 33-16: Add missing field }; builder.apply_to_boundary(&mut boundary) diff --git a/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs b/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs index 508e9f24..5ed0cd02 100644 --- a/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs +++ b/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs @@ -197,6 +197,7 @@ impl MirBuilder { boundary.condition_bindings = condition_bindings; boundary.exit_bindings = exit_bindings.clone(); boundary.expr_result = fragment_meta.expr_result; // Phase 33-14: Pass expr_result to merger + boundary.loop_var_name = Some(loop_var_name.clone()); // Phase 33-16: For LoopHeaderPhiBuilder // Phase 189: Capture exit PHI result (now used for reconnect) let _ = self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?; diff --git a/src/mir/builder/joinir_inline_boundary_injector.rs b/src/mir/builder/joinir_inline_boundary_injector.rs index a510a2da..61fa8cd3 100644 --- a/src/mir/builder/joinir_inline_boundary_injector.rs +++ b/src/mir/builder/joinir_inline_boundary_injector.rs @@ -26,6 +26,13 @@ impl BoundaryInjector { /// /// * `Ok(())` - 成功 /// * `Err(String)` - エラー（ブロックが見つからない等） + /// + /// # Phase 33-16: Loop Variable Header PHI Support + /// + /// When `boundary.loop_var_name` is set, the first join_input (loop variable) + /// is handled by the header PHI instead of a Copy instruction. We skip + /// emitting the Copy for join_inputs[0] in this case to avoid overwriting + /// the PHI result with the initial value. pub fn inject_boundary_copies( func: &mut MirFunction, entry_block_id: BasicBlockId, @@ -33,19 +40,28 @@ impl BoundaryInjector { value_map: &HashMap, debug: bool, ) -> Result<(), String> { + // Phase 33-16: When loop_var_name is set, skip first join_input (handled by header PHI) + let skip_first_join_input = boundary.loop_var_name.is_some(); + // Phase 171-fix: Check both join_inputs and condition_bindings - let total_inputs = boundary.join_inputs.len() + boundary.condition_bindings.len(); + let effective_join_inputs = if skip_first_join_input { + boundary.join_inputs.len().saturating_sub(1) + } else { + boundary.join_inputs.len() + }; + let total_inputs = effective_join_inputs + boundary.condition_bindings.len(); if total_inputs == 0 { return Ok(()); } if debug { eprintln!( - "[BoundaryInjector] Phase 171-fix: Injecting {} Copy instructions ({} join_inputs, {} condition_bindings) at entry block {:?}", + "[BoundaryInjector] Phase 171-fix: Injecting {} Copy instructions ({} join_inputs, {} condition_bindings) at entry block {:?}{}", total_inputs, - boundary.join_inputs.len(), + effective_join_inputs, boundary.condition_bindings.len(), - entry_block_id + entry_block_id, + if skip_first_join_input { " (skipping loop var - handled by header PHI)" } else { "" } ); } @@ -58,10 +74,13 @@ impl BoundaryInjector { let mut copy_instructions = Vec::new(); // Phase 171: Inject Copy instructions for join_inputs (loop parameters) + // Phase 33-16: Skip first join_input when loop_var_name is set (header PHI handles it) + let skip_count = if skip_first_join_input { 1 } else { 0 }; for (join_input, host_input) in boundary .join_inputs .iter() - .zip(boundary.host_inputs.iter()) + .skip(skip_count) + .zip(boundary.host_inputs.iter().skip(skip_count)) { // リマップ後の ValueId を取得 let remapped_join = value_map.get(join_input).copied().unwrap_or(*join_input); diff --git a/src/mir/join_ir/lowering/inline_boundary.rs b/src/mir/join_ir/lowering/inline_boundary.rs index 3a6f95eb..d711951b 100644 --- a/src/mir/join_ir/lowering/inline_boundary.rs +++ b/src/mir/join_ir/lowering/inline_boundary.rs @@ -232,6 +232,12 @@ pub struct JoinInlineBoundary { /// /// Here, `expr_result = None` because the loop doesn't return a value. pub expr_result: Option, + + /// Phase 33-16: Loop variable name (for LoopHeaderPhiBuilder) + /// + /// The name of the loop control variable (e.g., "i" in `loop(i < 3)`). + /// Used to track which PHI corresponds to the loop variable. + pub loop_var_name: Option, } impl JoinInlineBoundary { @@ -257,6 +263,7 @@ impl JoinInlineBoundary { condition_inputs: vec![], // Phase 171: Default to empty (deprecated) condition_bindings: vec![], // Phase 171-fix: Default to empty expr_result: None, // Phase 33-14: Default to carrier-only pattern + loop_var_name: None, // Phase 33-16 } } @@ -297,6 +304,7 @@ impl JoinInlineBoundary { condition_inputs: vec![], // Phase 171: Default to empty (deprecated) condition_bindings: vec![], // Phase 171-fix: Default to empty expr_result: None, // Phase 33-14 + loop_var_name: None, // Phase 33-16 } } @@ -333,6 +341,7 @@ impl JoinInlineBoundary { condition_inputs: vec![], // Phase 171: Default to empty (deprecated) condition_bindings: vec![], // Phase 171-fix: Default to empty expr_result: None, // Phase 33-14 + loop_var_name: None, // Phase 33-16 } } @@ -391,6 +400,7 @@ impl JoinInlineBoundary { condition_inputs: vec![], // Phase 171: Default to empty (deprecated) condition_bindings: vec![], // Phase 171-fix: Default to empty expr_result: None, // Phase 33-14 + loop_var_name: None, // Phase 33-16 } } @@ -435,6 +445,7 @@ impl JoinInlineBoundary { condition_inputs, condition_bindings: vec![], // Phase 171-fix: Will be populated by new constructor expr_result: None, // Phase 33-14 + loop_var_name: None, // Phase 33-16 } } @@ -483,6 +494,7 @@ impl JoinInlineBoundary { condition_inputs, condition_bindings: vec![], // Phase 171-fix: Will be populated by new constructor expr_result: None, // Phase 33-14 + loop_var_name: None, // Phase 33-16 } } @@ -538,6 +550,7 @@ impl JoinInlineBoundary { condition_inputs: vec![], // Deprecated, use condition_bindings instead condition_bindings, expr_result: None, // Phase 33-14 + loop_var_name: None, // Phase 33-16 } } }