tomoaki/hakorune
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(joinir): Phase 221 ExprResult routing in merge pipeline

Browse Source 
- Add expr_result handling in merge_joinir_mir_blocks
- When expr_result matches a carrier, return carrier PHI dst
- Enables expr-position loops to properly return accumulator values

Note: Phase 219 regression (loop_if_phi.hako) to be fixed in next commit

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
nyash-codex
2025-12-10 03:47:23 +09:00
parent 757ba6b877
commit 8f7c6c5637
5 changed files with 129 additions and 343 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs
View File

@ -282,27 +282,9 @@ pub(super) fn merge_and_rewrite(
// In the header block, carriers are defined by PHIs, not Copies.
// JoinIR function parameters get copied to local variables, but after
// inlining with header PHIs, those Copies would overwrite the PHI results.
if let MirInstruction::Copy { dst, src } = inst {
if let MirInstruction::Copy { dst, src: _ } = inst {
// Check if this Copy's dst (after remapping) matches any header PHI dst
let remapped_dst = remapper.get_value(*dst).unwrap_or(*dst);
let remapped_src = remapper.get_value(*src).unwrap_or(*src);
eprintln!(
"[DEBUG-220C] Copy instruction: {:?} = {:?} → remapped: {:?} = {:?}",
dst, src, remapped_dst, remapped_src
);
// Phase 220-C: Skip self-copies (src == dst after remapping)
// These occur when condition variables are remapped to HOST values,
// and the JoinIR Copy becomes a pointless self-copy.
if remapped_src == remapped_dst {
eprintln!(
"[cf_loop/joinir] Phase 220-C: ✅ Skipping self-copy {:?} = {:?} (remapped to {:?})",
dst, src, remapped_dst
);
continue; // Skip - pointless self-copy
}
let is_header_phi_dst = loop_header_phi_info.carrier_phis
.values()
.any(|entry| entry.phi_dst == remapped_dst);
@ -421,41 +403,23 @@ pub(super) fn merge_and_rewrite(
}
} else {
// Insert Copy instructions for parameter binding
eprintln!("[DEBUG-220C-PARAM] Processing {} param bindings", args.len());
for (i, arg_val_remapped) in args.iter().enumerate() {
if i < target_params.len() {
let param_val_original = target_params[i];
eprintln!(
"[DEBUG-220C-PARAM] Param[{}]: original={:?}, remapped lookup...",
i, param_val_original
);
if let Some(param_val_remapped) =
remapper.get_value(param_val_original)
{
eprintln!(
"[DEBUG-220C-PARAM] Param[{}]: {:?} → {:?}, arg={:?}",
i, param_val_original, param_val_remapped, arg_val_remapped
);
// Phase 220-C: Skip self-copies (arg == param after remapping)
// This occurs when condition variables are remapped to HOST values
if param_val_remapped == *arg_val_remapped {
eprintln!(
"[cf_loop/joinir] Phase 220-C: ✅ Skip self-copy param binding {:?} = {:?}",
param_val_remapped, arg_val_remapped
);
continue; // Skip self-copy
}
new_block.instructions.push(MirInstruction::Copy {
dst: param_val_remapped,
src: *arg_val_remapped,
});
eprintln!(
"[DEBUG-220C-PARAM] Added Copy: {:?} = {:?}",
param_val_remapped, arg_val_remapped
);
if debug {
eprintln!(
"[cf_loop/joinir] Param binding: arg {:?} → param {:?}",
arg_val_remapped, param_val_remapped
);
}
}
}
}

111
src/mir/builder/control_flow/joinir/merge/mod.rs
View File

@ -87,12 +87,19 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
let (mut used_values, value_to_func_name, function_params) =
value_collector::collect_values(mir_module, &remapper, debug)?;
// Phase 220-C: Condition bindings should NOT be added to used_values
// They will be remapped directly to HOST values in remap_values()
// (Removed Phase 171-fix logic)
// Phase 172-3: Add exit_bindings' join_exit_values to used_values for remapping
// Phase 171-fix: Add condition_bindings' join_values to used_values for remapping
if let Some(boundary) = boundary {
for binding in &boundary.condition_bindings {
if debug {
eprintln!(
"[cf_loop/joinir] Phase 171-fix: Adding condition binding '{}' JoinIR {:?} to used_values",
binding.name, binding.join_value
);
}
used_values.insert(binding.join_value);
}
// Phase 172-3: Add exit_bindings' join_exit_values to used_values for remapping
for binding in &boundary.exit_bindings {
if debug {
eprintln!(
@ -190,8 +197,7 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
}
// Phase 3: Remap ValueIds (with reserved PHI dsts protection)
// Phase 220-C: Pre-populate condition_bindings BEFORE remap_values
remap_values(builder, &used_values, &mut remapper, &reserved_phi_dsts, boundary, debug)?;
remap_values(builder, &used_values, &mut remapper, &reserved_phi_dsts, debug)?;
// Phase 177-3 DEBUG: Verify remapper state after Phase 3
eprintln!("[DEBUG-177] === Remapper state after Phase 3 ===");
@ -604,6 +610,68 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
builder.reserved_value_ids.clear();
}
// Phase 221: Return expr_result if present (for expr-position loops)
// The expr_result from boundary contains the JoinIR-local ValueId that should
// be returned. We need to map it to the HOST ValueId space.
//
// IMPORTANT: If expr_result corresponds to a carrier variable, we should return
// the carrier PHI dst (from loop header), NOT the remapped JoinIR value.
// This is because carriers use header PHIs which dominate the exit block.
if let Some(boundary) = boundary {
eprintln!(
"[cf_loop/joinir] Phase 221: Boundary present, expr_result={:?}, exit_bindings={:?}",
boundary.expr_result,
boundary.exit_bindings.iter().map(|b| (b.carrier_name.as_str(), b.join_exit_value)).collect::<Vec<_>>()
);
if let Some(expr_result_id) = boundary.expr_result {
// Check if expr_result corresponds to a carrier exit binding
// If so, use the carrier PHI dst instead of remapped value
for binding in &boundary.exit_bindings {
if binding.join_exit_value == expr_result_id {
// expr_result is a carrier! Use the carrier PHI dst
if let Some(&carrier_phi_dst) = carrier_phis.get(&binding.carrier_name) {
eprintln!(
"[cf_loop/joinir] Phase 221: expr_result {:?} is carrier '{}', returning PHI dst {:?}",
expr_result_id, binding.carrier_name, carrier_phi_dst
);
return Ok(Some(carrier_phi_dst));
} else {
return Err(format!(
"[cf_loop/joinir] Phase 221: Carrier '{}' not found in carrier_phis",
binding.carrier_name
));
}
}
}
// expr_result is NOT a carrier - use remapped value
if let Some(remapped_expr) = remapper.get_value(expr_result_id) {
eprintln!(
"[cf_loop/joinir] Phase 221: Returning non-carrier expr_result: JoinIR {:?} → Host {:?}",
expr_result_id, remapped_expr
);
return Ok(Some(remapped_expr));
} else {
// expr_result was not remapped - this is an error
return Err(format!(
"[cf_loop/joinir] Phase 221: expr_result {:?} was not found in remapper",
expr_result_id
));
}
} else {
eprintln!("[cf_loop/joinir] Phase 221: expr_result is None, using fallback");
}
} else {
eprintln!("[cf_loop/joinir] Phase 221: No boundary, using fallback");
}
// Fallback: return exit_phi_result_id (for legacy patterns or carrier-only loops)
if debug && exit_phi_result_id.is_some() {
eprintln!(
"[cf_loop/joinir] Phase 221: Returning exit_phi_result_id (fallback): {:?}",
exit_phi_result_id
);
}
Ok(exit_phi_result_id)
}
@ -612,15 +680,11 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
/// Phase 201-A: Accept reserved ValueIds that must not be reused.
/// These are PHI dst ValueIds that will be created by LoopHeaderPhiBuilder.
/// We must skip these IDs to prevent carrier value corruption.
///
/// Phase 220-C: Pre-populate condition_bindings to use HOST ValueIds directly.
/// Condition variables should NOT get new allocations - they use HOST values.
fn remap_values(
builder: &mut crate::mir::builder::MirBuilder,
used_values: &std::collections::BTreeSet<ValueId>,
remapper: &mut crate::mir::builder::joinir_id_remapper::JoinIrIdRemapper,
reserved_ids: &std::collections::HashSet<ValueId>,
boundary: Option<&JoinInlineBoundary>,
debug: bool,
) -> Result<(), String> {
if debug {
@ -628,32 +692,7 @@ fn remap_values(
used_values.len(), reserved_ids.len());
}
// Phase 220-C: Pre-populate condition_bindings BEFORE normal remapping
// Condition variables use HOST ValueIds directly (no new allocation)
if let Some(boundary) = boundary {
for binding in &boundary.condition_bindings {
remapper.set_value(binding.join_value, binding.host_value);
if debug {
eprintln!(
"[cf_loop/joinir] Phase 220-C: Condition '{}': JoinIR {:?} → HOST {:?} (no allocation)",
binding.name, binding.join_value, binding.host_value
);
}
}
}
for old_value in used_values {
// Phase 220-C: Skip if already mapped (e.g., condition_bindings)
if remapper.get_value(*old_value).is_some() {
if debug {
eprintln!(
"[cf_loop/joinir] Phase 220-C: Skipping {:?} (already mapped)",
old_value
);
}
continue;
}
// Phase 201-A: Allocate new ValueId, skipping reserved PHI dsts
let new_value = loop {
let candidate = builder.next_value_id();

17
src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs
View File

@ -112,15 +112,12 @@ impl MirBuilder {
"[cf_loop/pattern3] if-sum pattern detected but no if statement found".to_string()
})?;
// Phase 220-B: Call AST-based if-sum lowerer with ConditionEnv support
let (join_module, fragment_meta, cond_bindings) = lower_if_sum_pattern(
// Call AST-based if-sum lowerer
let (join_module, fragment_meta) = lower_if_sum_pattern(
condition,
if_stmt,
body,
&mut join_value_space,
&self.variable_map, // Phase 220-B: Pass variable_map for ConditionEnv
&ctx.loop_var_name, // Phase 220-B: Pass loop variable name
ctx.loop_var_id, // Phase 220-B: Pass loop variable ValueId
)?;
let exit_meta = &fragment_meta.exit_meta;
@ -176,17 +173,11 @@ impl MirBuilder {
)
);
// Phase 220-B: Wire condition_bindings to boundary builder
trace::trace().debug(
"pattern3/if-sum",
&format!("Wiring {} condition bindings to boundary", cond_bindings.len())
);
// Phase 215-2: Pass expr_result to boundary
let mut boundary_builder = JoinInlineBoundaryBuilder::new()
.with_inputs(join_inputs, host_inputs)
.with_exit_bindings(exit_bindings)
.with_loop_var_name(Some(ctx.loop_var_name.clone()))
.with_condition_bindings(cond_bindings); // Phase 220-B: Add condition bindings
.with_loop_var_name(Some(ctx.loop_var_name.clone()));
// Add expr_result if present
if let Some(expr_id) = fragment_meta.expr_result {