feat(joinir): Phase 246-EX Part 2 - Exit PHI & step scheduling fixes
Phase 246-EX Part 2 completes the _atoi JoinIR integration: Key fixes: - Exit PHI connection: ExitLineReconnector now correctly uses exit PHI dsts - Jump args preservation: BasicBlock.jump_args field stores JoinIR exit values - instruction_rewriter: Reads jump_args, remaps JoinIR→HOST values by carrier order - step_schedule.rs: New module for body-local init step ordering Files changed: - reconnector.rs: Exit PHI connection improvements - instruction_rewriter.rs: Jump args reading & carrier value mapping - loop_with_break_minimal.rs: Refactored step scheduling - step_schedule.rs: NEW - Step ordering logic extracted Tests: 931/931 PASS (no regression) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@ -75,6 +75,8 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
|
||||
boundary: Option<&JoinInlineBoundary>,
|
||||
debug: bool,
|
||||
) -> Result<Option<ValueId>, String> {
|
||||
let verbose = debug || crate::config::env::joinir_dev_enabled();
|
||||
|
||||
if debug {
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] merge_joinir_mir_blocks called with {} functions",
|
||||
@ -82,6 +84,54 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
|
||||
);
|
||||
}
|
||||
|
||||
if verbose {
|
||||
if let Some(boundary) = boundary {
|
||||
let exit_summary: Vec<String> = boundary
|
||||
.exit_bindings
|
||||
.iter()
|
||||
.map(|b| {
|
||||
format!(
|
||||
"{}: join {:?} → host {:?} ({:?})",
|
||||
b.carrier_name, b.join_exit_value, b.host_slot, b.role
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
|
||||
let cond_summary: Vec<String> = boundary
|
||||
.condition_bindings
|
||||
.iter()
|
||||
.map(|b| format!("{}: host {:?} → join {:?}", b.name, b.host_value, b.join_value))
|
||||
.collect();
|
||||
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] Boundary join_inputs={:?} host_inputs={:?}",
|
||||
boundary.join_inputs, boundary.host_inputs
|
||||
);
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] Boundary exit_bindings ({}): {}",
|
||||
boundary.exit_bindings.len(),
|
||||
exit_summary.join(", ")
|
||||
);
|
||||
if !cond_summary.is_empty() {
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] Boundary condition_bindings ({}): {}",
|
||||
cond_summary.len(),
|
||||
cond_summary.join(", ")
|
||||
);
|
||||
}
|
||||
if let Some(ci) = &boundary.carrier_info {
|
||||
let carriers: Vec<String> = ci.carriers.iter().map(|c| c.name.clone()).collect();
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] Boundary carrier_info: loop_var='{}', carriers={:?}",
|
||||
ci.loop_var_name,
|
||||
carriers
|
||||
);
|
||||
}
|
||||
} else {
|
||||
eprintln!("[cf_loop/joinir] No boundary provided");
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 1: Allocate block IDs for all functions
|
||||
// Phase 177-3: block_allocator now returns exit_block_id to avoid conflicts
|
||||
let (mut remapper, exit_block_id) = block_allocator::allocate_blocks(builder, mir_module, debug)?;
|
||||
@ -529,7 +579,8 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
|
||||
|
||||
// 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(
|
||||
// Phase 246-EX: REVERT Phase 33-20 - Use EXIT PHI dsts, not header PHI dsts!
|
||||
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,
|
||||
@ -537,18 +588,32 @@ 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();
|
||||
// Phase 246-EX: CRITICAL FIX - Use exit PHI dsts for variable_map reconnection
|
||||
//
|
||||
// **Why EXIT PHI, not HEADER PHI?**
|
||||
//
|
||||
// Header PHI represents the value at the BEGINNING of each iteration.
|
||||
// Exit PHI represents the FINAL value when leaving the loop (from any exit path).
|
||||
//
|
||||
// For Pattern 2 loops with multiple exit paths (natural exit + break):
|
||||
// - Header PHI: `%15 = phi [%3, bb7], [%42, bb14]` (loop variable at iteration start)
|
||||
// - Exit PHI: `%5 = phi [%15, bb11], [%15, bb13]` (final value from exit paths)
|
||||
//
|
||||
// When we exit the loop, we want the FINAL value (%5), not the iteration-start value (%15).
|
||||
// Phase 33-20 incorrectly used header PHI, causing loops to return initial values (e.g., 0 instead of 42).
|
||||
//
|
||||
// Example (_atoi):
|
||||
// - Initial: result=0 (header PHI)
|
||||
// - After iteration 1: result=4 (updated in loop body)
|
||||
// - After iteration 2: result=42 (updated in loop body)
|
||||
// - Exit: Should return 42 (exit PHI), not 0 (header PHI initial value)
|
||||
//
|
||||
// The exit PHI correctly merges values from both exit paths, giving us the final result.
|
||||
let carrier_phis = &exit_carrier_phis;
|
||||
|
||||
if debug && !carrier_phis.is_empty() {
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] Phase 33-20: Using header PHI dsts for variable_map: {:?}",
|
||||
"[cf_loop/joinir] Phase 246-EX: Using EXIT PHI dsts for variable_map (not header): {:?}",
|
||||
carrier_phis.iter().map(|(n, v)| (n.as_str(), v)).collect::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
@ -556,9 +621,9 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
|
||||
// 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-20: Now uses header PHI dsts instead of exit PHI dsts
|
||||
// Phase 246-EX: Now uses EXIT PHI dsts (reverted Phase 33-20)
|
||||
if let Some(boundary) = boundary {
|
||||
exit_line::ExitLineOrchestrator::execute(builder, boundary, &carrier_phis, debug)?;
|
||||
exit_line::ExitLineOrchestrator::execute(builder, boundary, carrier_phis, debug)?;
|
||||
}
|
||||
|
||||
let exit_block_id = merge_result.exit_block_id;
|
||||
@ -624,19 +689,69 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
|
||||
builder.reserved_value_ids.clear();
|
||||
}
|
||||
|
||||
// Phase 221-R: Use ExprResultResolver Box
|
||||
let expr_result_value = expr_result_resolver::ExprResultResolver::resolve(
|
||||
boundary.and_then(|b| b.expr_result),
|
||||
boundary.map(|b| b.exit_bindings.as_slice()).unwrap_or(&[]),
|
||||
&carrier_phis,
|
||||
&remapper,
|
||||
debug,
|
||||
)?;
|
||||
// Phase 246-EX-FIX: Handle loop variable expr_result separately from carrier expr_result
|
||||
//
|
||||
// The loop variable (e.g., 'i') is returned via exit_phi_result_id, not carrier_phis.
|
||||
// Other carriers use carrier_phis. We need to check which case we're in.
|
||||
let expr_result_value = if let Some(b) = boundary {
|
||||
if let Some(expr_result_id) = b.expr_result {
|
||||
// Check if expr_result is the loop variable
|
||||
if let Some(loop_var_name) = &b.loop_var_name {
|
||||
// Find the exit binding for the loop variable
|
||||
let loop_var_binding = b.exit_bindings.iter()
|
||||
.find(|binding| binding.carrier_name == *loop_var_name);
|
||||
|
||||
if let Some(binding) = loop_var_binding {
|
||||
if binding.join_exit_value == expr_result_id {
|
||||
// expr_result is the loop variable! Use exit_phi_result_id
|
||||
if debug {
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] Phase 246-EX-FIX: expr_result {:?} is loop variable '{}', using exit_phi_result_id {:?}",
|
||||
expr_result_id, loop_var_name, exit_phi_result_id
|
||||
);
|
||||
}
|
||||
exit_phi_result_id
|
||||
} else {
|
||||
// expr_result is not the loop variable, resolve as carrier
|
||||
expr_result_resolver::ExprResultResolver::resolve(
|
||||
Some(expr_result_id),
|
||||
b.exit_bindings.as_slice(),
|
||||
&carrier_phis,
|
||||
&remapper,
|
||||
debug,
|
||||
)?
|
||||
}
|
||||
} else {
|
||||
// No loop variable binding, resolve normally
|
||||
expr_result_resolver::ExprResultResolver::resolve(
|
||||
Some(expr_result_id),
|
||||
b.exit_bindings.as_slice(),
|
||||
&carrier_phis,
|
||||
&remapper,
|
||||
debug,
|
||||
)?
|
||||
}
|
||||
} else {
|
||||
// No loop variable name, resolve normally
|
||||
expr_result_resolver::ExprResultResolver::resolve(
|
||||
Some(expr_result_id),
|
||||
b.exit_bindings.as_slice(),
|
||||
&carrier_phis,
|
||||
&remapper,
|
||||
debug,
|
||||
)?
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
// Return expr_result if present, otherwise fall back to exit_phi_result_id
|
||||
if let Some(resolved) = expr_result_value {
|
||||
if debug {
|
||||
eprintln!("[cf_loop/joinir] Phase 221-R: Returning expr_result_value {:?}", resolved);
|
||||
eprintln!("[cf_loop/joinir] Phase 246-EX-FIX: Returning expr_result_value {:?}", resolved);
|
||||
}
|
||||
Ok(Some(resolved))
|
||||
} else {
|
||||
|
||||
Reference in New Issue
Block a user