fix(joinir): Phase 189 exit PHI to variable_map connection for Pattern 3
Pattern 3 (loop + if-else PHI) was returning 0 instead of the correct sum (9). Root cause: JoinIR k_exit function returned the final sum value, and the Return→Jump conversion collected these into an exit block PHI, but the host's variable_map["sum"] still pointed to the original ValueId (initial 0). Fix: - Changed merge_joinir_mir_blocks() return type from Result<(), String> to Result<Option<ValueId>, String> to return exit PHI result - Pattern 3 now updates variable_map["sum"] with the exit PHI ValueId - Patterns 1/2 discard the exit PHI result (they return void) Test: sum of odd numbers 1+3+5 = 9 now correctly returned 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
nyash-codex
@ -93,6 +93,8 @@ impl super::MirBuilder {
|
||||
.map(|f| f.signature.name.clone())
|
||||
.unwrap_or_default();
|
||||
|
||||
eprintln!("[cf_loop/joinir/router] try_cf_loop_joinir called for function: '{}'", func_name);
|
||||
|
||||
// Phase 188-Impl-3: Debug logging for function name routing
|
||||
if std::env::var("NYASH_JOINIR_MAINLINE_DEBUG").is_ok() {
|
||||
eprintln!("[cf_loop/joinir/router] Current function name: '{}'", func_name);
|
||||
@ -137,6 +139,10 @@ impl super::MirBuilder {
|
||||
// Debug log when routing through JoinIR Frontend
|
||||
let debug = std::env::var("NYASH_LOOPFORM_DEBUG").is_ok()
|
||||
|| std::env::var("NYASH_JOINIR_MAINLINE_DEBUG").is_ok();
|
||||
eprintln!("[cf_loop/joinir] DEBUG FLAG STATUS: debug={}, NYASH_LOOPFORM_DEBUG={:?}, NYASH_JOINIR_MAINLINE_DEBUG={:?}",
|
||||
debug,
|
||||
std::env::var("NYASH_LOOPFORM_DEBUG"),
|
||||
std::env::var("NYASH_JOINIR_MAINLINE_DEBUG"));
|
||||
if debug {
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] Routing {} through JoinIR Frontend mainline",
|
||||
@ -383,7 +389,8 @@ impl super::MirBuilder {
|
||||
// - Jump from current_block to the entry of generated loop
|
||||
// - The loop exit becomes the new current_block
|
||||
// Phase 188-Impl-3: Pass None for boundary (legacy path without boundary)
|
||||
self.merge_joinir_mir_blocks(&mir_module, None, debug)?;
|
||||
// Phase 189: Discard exit PHI result (legacy path doesn't need it)
|
||||
let _ = self.merge_joinir_mir_blocks(&mir_module, None, debug)?;
|
||||
|
||||
// Return void for now (loop doesn't have a meaningful return value in this context)
|
||||
let void_val = self.next_value_id();
|
||||
@ -514,7 +521,8 @@ impl super::MirBuilder {
|
||||
vec![ValueId(0)], // JoinIR's main() parameter (loop variable)
|
||||
vec![loop_var_id], // Host's loop variable
|
||||
);
|
||||
self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?;
|
||||
// Phase 189: Discard exit PHI result (Pattern 1 returns void)
|
||||
let _ = self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?;
|
||||
|
||||
// Phase 188-Impl-4-FIX: Return Void instead of trying to emit Const
|
||||
//
|
||||
@ -646,7 +654,8 @@ impl super::MirBuilder {
|
||||
vec![ValueId(0)], // JoinIR's main() parameter (loop variable init)
|
||||
vec![loop_var_id], // Host's loop variable
|
||||
);
|
||||
self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?;
|
||||
// Phase 189: Discard exit PHI result (Pattern 2 returns void)
|
||||
let _ = self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?;
|
||||
|
||||
// Phase 188-Impl-2: Return Void (loops don't produce values)
|
||||
// The subsequent "return i" statement will emit its own Load + Return
|
||||
@ -778,9 +787,23 @@ impl super::MirBuilder {
|
||||
vec![ValueId(0), ValueId(1)], // JoinIR's main() parameters (i, sum init)
|
||||
vec![loop_var_id, sum_var_id], // Host's loop variables
|
||||
);
|
||||
self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?;
|
||||
let exit_phi_result = self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?;
|
||||
|
||||
// Phase 188-Impl-3: Return Void (loops don't produce values)
|
||||
// Phase 189-Fix: Update variable_map["sum"] to point to exit PHI result
|
||||
// The exit PHI contains the final value of sum after the loop completes.
|
||||
// This ensures subsequent references to "sum" (e.g., `return sum`) use the correct value.
|
||||
if let Some(exit_phi) = exit_phi_result {
|
||||
self.variable_map.insert("sum".to_string(), exit_phi);
|
||||
if debug {
|
||||
eprintln!(
|
||||
"[cf_loop/joinir/pattern3] Updated variable_map['sum'] to exit PHI {:?}",
|
||||
exit_phi
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 188-Impl-3: Return Void (the loop itself doesn't produce a value, but its result
|
||||
// is now accessible via variable_map["sum"])
|
||||
let void_val = crate::mir::builder::emission::constant::emit_void(self);
|
||||
|
||||
if debug {
|
||||
@ -823,12 +846,18 @@ impl super::MirBuilder {
|
||||
///
|
||||
/// This enables clean separation: JoinIR uses local IDs (0,1,2...),
|
||||
/// host uses its own IDs, and Copy instructions bridge the gap.
|
||||
///
|
||||
/// # Returns
|
||||
///
|
||||
/// Returns `Ok(Some(exit_phi_id))` if the merged JoinIR functions have return values
|
||||
/// that were collected into an exit block PHI. This allows the caller to connect
|
||||
/// the exit PHI result to the appropriate host variable.
|
||||
fn merge_joinir_mir_blocks(
|
||||
&mut self,
|
||||
mir_module: &crate::mir::MirModule,
|
||||
boundary: Option<&crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary>,
|
||||
debug: bool,
|
||||
) -> Result<(), String> {
|
||||
) -> Result<Option<ValueId>, String> {
|
||||
use crate::mir::{BasicBlock, BasicBlockId, MirInstruction, ValueId};
|
||||
use std::collections::HashMap;
|
||||
// Phase 189: Use new ID remapper Box
|
||||
@ -916,6 +945,9 @@ impl super::MirBuilder {
|
||||
if let crate::mir::types::ConstValue::String(s) = value {
|
||||
if function_entry_map.contains_key(s) {
|
||||
value_to_func_name.insert(*dst, s.clone());
|
||||
// Phase 189 FIX: Also add to used_values so it gets remapped!
|
||||
// Without this, subsequent instructions referencing dst will fail
|
||||
used_values.insert(*dst);
|
||||
if debug {
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] Found function name constant: {:?} = '{}'",
|
||||
@ -953,6 +985,18 @@ impl super::MirBuilder {
|
||||
// Phase 189: Iterate over both names and functions (need name for composite keys)
|
||||
let mut functions_merge: Vec<_> = mir_module.functions.iter().collect();
|
||||
functions_merge.sort_by_key(|(name, _)| name.as_str());
|
||||
|
||||
// Phase 189 FIX: Build set of boundary join_inputs to skip their Const initializers
|
||||
// When BoundaryInjector provides values via Copy, entry function shouldn't also set them via Const
|
||||
let boundary_input_set: std::collections::HashSet<ValueId> = boundary
|
||||
.map(|b| b.join_inputs.iter().copied().collect())
|
||||
.unwrap_or_default();
|
||||
let entry_func_name = functions_merge.first().map(|(name, _)| name.as_str());
|
||||
|
||||
// Phase 189-Fix: Collect return values from JoinIR functions for exit PHI
|
||||
// Each Return → Jump conversion records (from_block, return_value) here
|
||||
let mut exit_phi_inputs: Vec<(BasicBlockId, ValueId)> = Vec::new();
|
||||
|
||||
for (func_name, func) in functions_merge {
|
||||
if debug {
|
||||
eprintln!(
|
||||
@ -985,6 +1029,20 @@ impl super::MirBuilder {
|
||||
let mut found_tail_call = false;
|
||||
let mut tail_call_target: Option<(BasicBlockId, Vec<ValueId>)> = None;
|
||||
|
||||
// DEBUG: Print block being processed
|
||||
if debug {
|
||||
eprintln!("[cf_loop/joinir] === Processing block {:?} (from func '{}') ===", old_block_id, func_name);
|
||||
eprintln!("[cf_loop/joinir] Original block has {} instructions:", old_block.instructions.len());
|
||||
for (idx, inst) in old_block.instructions.iter().enumerate() {
|
||||
eprintln!("[cf_loop/joinir] [{}] {:?}", idx, inst);
|
||||
}
|
||||
eprintln!("[cf_loop/joinir] Original block terminator: {:?}", old_block.terminator);
|
||||
}
|
||||
|
||||
// Phase 189 FIX: Check if this is entry function's entry block (for boundary input skipping)
|
||||
let is_entry_func_entry_block = entry_func_name == Some(func_name.as_str())
|
||||
&& *old_block_id == func.entry_block;
|
||||
|
||||
// First pass: Process all instructions, identify tail calls
|
||||
for inst in &old_block.instructions {
|
||||
// Phase 189: Skip Const String instructions that define function names
|
||||
@ -997,6 +1055,14 @@ impl super::MirBuilder {
|
||||
continue; // Skip this instruction
|
||||
}
|
||||
}
|
||||
// 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 debug {
|
||||
eprintln!("[cf_loop/joinir] Skipping boundary input const (replaced by BoundaryInjector Copy): {:?}", inst);
|
||||
}
|
||||
continue; // Skip - BoundaryInjector will provide the value
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 189: Detect tail calls and save parameters
|
||||
@ -1058,6 +1124,14 @@ impl super::MirBuilder {
|
||||
new_block.instructions.push(remapped_with_blocks);
|
||||
}
|
||||
|
||||
// DEBUG: Print what was added to the block after first pass
|
||||
if debug {
|
||||
eprintln!("[cf_loop/joinir] After first pass, new_block has {} instructions", new_block.instructions.len());
|
||||
for (idx, inst) in new_block.instructions.iter().enumerate() {
|
||||
eprintln!("[cf_loop/joinir] [{}] {:?}", idx, inst);
|
||||
}
|
||||
}
|
||||
|
||||
// Second pass: Insert parameter bindings for tail calls
|
||||
// Phase 188-Impl-3: Use actual parameter ValueIds from target function
|
||||
if let Some((target_block, args)) = tail_call_target {
|
||||
@ -1106,9 +1180,25 @@ impl super::MirBuilder {
|
||||
new_block.terminator = Some(MirInstruction::Jump {
|
||||
target: target_block,
|
||||
});
|
||||
|
||||
// DEBUG: Print final state after adding parameter bindings
|
||||
if debug {
|
||||
eprintln!("[cf_loop/joinir] After adding param bindings, new_block has {} instructions", new_block.instructions.len());
|
||||
for (idx, inst) in new_block.instructions.iter().enumerate() {
|
||||
eprintln!("[cf_loop/joinir] [{}] {:?}", idx, inst);
|
||||
}
|
||||
}
|
||||
}
|
||||
new_block.instruction_spans = old_block.instruction_spans.clone();
|
||||
|
||||
if debug {
|
||||
eprintln!("[cf_loop/joinir] Span sync: new_block.instructions.len()={}, old_block.instruction_spans.len()={}, new_block.instruction_spans.len()={}",
|
||||
new_block.instructions.len(),
|
||||
old_block.instruction_spans.len(),
|
||||
new_block.instruction_spans.len()
|
||||
);
|
||||
}
|
||||
|
||||
// Remap terminator (convert Return → Jump to exit) if not already set by tail call
|
||||
if !found_tail_call {
|
||||
if let Some(ref term) = old_block.terminator {
|
||||
@ -1116,6 +1206,7 @@ impl super::MirBuilder {
|
||||
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
|
||||
if let Some(ret_val) = value {
|
||||
let remapped_val = remapper.get_value(*ret_val).unwrap_or(*ret_val);
|
||||
if debug {
|
||||
@ -1124,6 +1215,8 @@ impl super::MirBuilder {
|
||||
remapped_val
|
||||
);
|
||||
}
|
||||
// Collect (from_block, return_value) for exit PHI generation
|
||||
exit_phi_inputs.push((new_block_id, remapped_val));
|
||||
}
|
||||
MirInstruction::Jump {
|
||||
target: exit_block_id,
|
||||
@ -1136,17 +1229,11 @@ impl super::MirBuilder {
|
||||
}
|
||||
}
|
||||
MirInstruction::Branch { condition, then_bb, else_bb } => {
|
||||
// Phase 189 FIX: Remap block IDs for Branch
|
||||
let remapped = remapper.remap_instruction(term);
|
||||
match remapped {
|
||||
MirInstruction::Branch { condition, then_bb, else_bb } => {
|
||||
MirInstruction::Branch {
|
||||
condition,
|
||||
then_bb: local_block_map.get(&then_bb).copied().unwrap_or(then_bb),
|
||||
else_bb: local_block_map.get(&else_bb).copied().unwrap_or(else_bb),
|
||||
}
|
||||
}
|
||||
other => other,
|
||||
// Phase 189 FIX: Remap block IDs AND condition ValueId for Branch
|
||||
MirInstruction::Branch {
|
||||
condition: remapper.remap_value(*condition),
|
||||
then_bb: local_block_map.get(then_bb).copied().unwrap_or(*then_bb),
|
||||
else_bb: local_block_map.get(else_bb).copied().unwrap_or(*else_bb),
|
||||
}
|
||||
}
|
||||
_ => remapper.remap_instruction(term),
|
||||
@ -1155,6 +1242,22 @@ impl super::MirBuilder {
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 189 FIX: Ensure instruction_spans matches instructions length
|
||||
// The original spans may not cover all instructions after remapping/adding
|
||||
// (PHI instructions, tail call parameter bindings, etc.)
|
||||
let inst_count = new_block.instructions.len();
|
||||
let span_count = new_block.instruction_spans.len();
|
||||
if inst_count > span_count {
|
||||
// Use a default span for the extra instructions
|
||||
let default_span = new_block.instruction_spans.last()
|
||||
.copied()
|
||||
.unwrap_or_else(crate::ast::Span::unknown);
|
||||
new_block.instruction_spans.resize(inst_count, default_span);
|
||||
} else if inst_count < span_count {
|
||||
// Truncate spans to match instructions
|
||||
new_block.instruction_spans.truncate(inst_count);
|
||||
}
|
||||
|
||||
// Add block to current function
|
||||
if let Some(ref mut current_func) = self.current_function {
|
||||
current_func.add_block(new_block);
|
||||
@ -1195,10 +1298,59 @@ impl super::MirBuilder {
|
||||
}
|
||||
}
|
||||
|
||||
// 6. Create exit block (empty for now, will be populated after loop)
|
||||
if let Some(ref mut func) = self.current_function {
|
||||
let exit_block = BasicBlock::new(exit_block_id);
|
||||
// 6. Create exit block with PHI for return values from JoinIR functions
|
||||
// Phase 189-Fix: Generate exit PHI if there are multiple return values
|
||||
let exit_phi_result_id = if let Some(ref mut func) = self.current_function {
|
||||
let mut exit_block = BasicBlock::new(exit_block_id);
|
||||
|
||||
// Phase 189-Fix: If we collected return values, create a PHI in exit block
|
||||
// This merges all return values from JoinIR functions into a single value
|
||||
let phi_result = if !exit_phi_inputs.is_empty() {
|
||||
// Allocate a new ValueId for the PHI result
|
||||
let phi_dst = self.value_gen.next();
|
||||
exit_block.instructions.push(MirInstruction::Phi {
|
||||
dst: phi_dst,
|
||||
inputs: exit_phi_inputs.clone(),
|
||||
type_hint: None,
|
||||
});
|
||||
exit_block.instruction_spans.push(crate::ast::Span::unknown());
|
||||
if debug {
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] Exit block PHI: {:?} = phi {:?}",
|
||||
phi_dst, exit_phi_inputs
|
||||
);
|
||||
}
|
||||
Some(phi_dst)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
func.add_block(exit_block);
|
||||
if debug {
|
||||
eprintln!("[cf_loop/joinir] Created exit block: {:?}", exit_block_id);
|
||||
}
|
||||
phi_result
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
// Phase 189-Fix: Store exit PHI result in variable_map so host code can reference it
|
||||
// The loop result should update the corresponding carrier variable
|
||||
if let Some(phi_result) = exit_phi_result_id {
|
||||
// Use boundary output info to connect exit PHI to host variables
|
||||
if let Some(ref boundary) = boundary {
|
||||
// If boundary has outputs, map JoinIR output to host output
|
||||
if !boundary.join_outputs.is_empty() && !boundary.host_outputs.is_empty() {
|
||||
// For now, we assume single output (sum in the pattern)
|
||||
// Future: support multiple outputs
|
||||
if debug {
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] Would connect exit PHI {:?} to host output {:?}",
|
||||
phi_result, boundary.host_outputs
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// 7. Jump from current block to entry function's entry block
|
||||
@ -1212,9 +1364,16 @@ impl super::MirBuilder {
|
||||
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))?;
|
||||
if debug {
|
||||
eprintln!("[cf_loop/joinir] Entry function name: {}", entry_func_name);
|
||||
eprintln!("[cf_loop/joinir] Entry function's entry_block (JoinIR local): {:?}", entry_func.entry_block);
|
||||
eprintln!("[cf_loop/joinir] Entry block (remapped): {:?}", entry_block);
|
||||
eprintln!("[cf_loop/joinir] Current block before emit_jump: {:?}", self.current_block);
|
||||
eprintln!("[cf_loop/joinir] Jumping to entry block: {:?}", entry_block);
|
||||
}
|
||||
crate::mir::builder::emission::branch::emit_jump(self, entry_block)?;
|
||||
if debug {
|
||||
eprintln!("[cf_loop/joinir] After emit_jump, current_block: {:?}", self.current_block);
|
||||
}
|
||||
|
||||
// 8. Switch to exit block for subsequent code
|
||||
self.start_new_block(exit_block_id)?;
|
||||
@ -1225,9 +1384,23 @@ impl super::MirBuilder {
|
||||
mir_module.functions.len(),
|
||||
exit_block_id
|
||||
);
|
||||
// DEBUG: Check bb0's terminator after merge
|
||||
if let Some(ref func) = self.current_function {
|
||||
if let Some(bb0) = func.get_block(BasicBlockId(0)) {
|
||||
eprintln!("[cf_loop/joinir] bb0 terminator after merge: {:?}", bb0.terminator);
|
||||
}
|
||||
// DEBUG: Check bb9's PHI after merge (PHI merge block)
|
||||
if let Some(bb9) = func.get_block(BasicBlockId(9)) {
|
||||
let phi_count = bb9.instructions.iter().filter(|i| matches!(i, MirInstruction::Phi { .. })).count();
|
||||
eprintln!("[cf_loop/joinir] bb9 after merge: {} instructions, {} PHI", bb9.instructions.len(), phi_count);
|
||||
for (idx, inst) in bb9.instructions.iter().take(3).enumerate() {
|
||||
eprintln!("[cf_loop/joinir] bb9[{}]: {:?}", idx, inst);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
Ok(exit_phi_result_id)
|
||||
}
|
||||
|
||||
// Phase 189: collect_values_in_block/collect_values_in_instruction removed
|
||||
|
||||
Reference in New Issue
Block a user