feat(joinir): Phase 33-21 Pattern 4 parameter remapping fix and debug improvements
## Problem Pattern 4 (loop with continue) was producing SSA-undef errors due to function_params key mismatch. function_params uses 'join_func_0'/'join_func_1' format (from join_func_name()), but code was looking for 'main'/'loop_step'. ## Solution - Fix mod.rs: Use correct function keys 'join_func_0' and 'join_func_1' - Add warning logs to detect future key mismatches immediately - Update pattern4_with_continue.rs: Mark as fully implemented (not stub) - Remove unused imports: MergeResult, TailCallKind, classify_tail_call, etc. ## Testing - Pattern 3 (If PHI): sum=9 ✅ - Pattern 4 (Continue): 25 ✅ (1+3+5+7+9) - No SSA-undef errors - No new warnings on successful execution ## Debug Improvements Added pre-flight checks in merge/mod.rs Phase 33-21: ``` [cf_loop/joinir] WARNING: function_params.get('join_func_0') returned None. Available keys: [...] ``` This will save significant debugging time for future parameter mapping issues. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
nyash-codex
@ -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::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
|
||||
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::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
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::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
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::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
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<String, ValueId> = 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::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
|
||||
// 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)?;
|
||||
}
|
||||
|
||||
@ -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<Option<ValueId>, 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::<Vec<_>>())
|
||||
);
|
||||
|
||||
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))
|
||||
}
|
||||
}
|
||||
|
||||
@ -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;
|
||||
|
||||
Reference in New Issue
Block a user