feat(joinir): Phase 33-14 JoinFragmentMeta for expr/carrier separation

Introduces JoinFragmentMeta to distinguish between loop expression results
and carrier variable updates, fixing SSA correctness issues.

## Changes

### New: JoinFragmentMeta struct (carrier_info.rs)
- `expr_result: Option<ValueId>` - Loop as expression (return loop(...))
- `exit_meta: ExitMeta` - Carrier updates for variable_map
- Helper methods: with_expr_result(), carrier_only(), empty()

### Pattern 2 Lowerer Updates
- loop_with_break_minimal.rs: Returns (JoinModule, JoinFragmentMeta)
- pattern2_with_break.rs: Sets boundary.expr_result from fragment_meta

### instruction_rewriter.rs
- Phase 33-14: Only add to exit_phi_inputs when boundary.expr_result is Some
- Phase 33-13: MergeResult struct with carrier_inputs map

### JoinInlineBoundary (inline_boundary.rs)
- New field: expr_result: Option<ValueId>
- All constructors updated with expr_result: None default

## Design Philosophy

Previously, exit_phi_inputs mixed expr results with carrier updates, causing:
- PHI inputs referencing undefined remapped values
- SSA-undef errors in VM execution

With JoinFragmentMeta:
- expr_result → exit_phi_inputs (generates PHI for expr value)
- exit_meta → carrier_inputs (updates variable_map via carrier PHIs)

## Test Results
- Pattern 1 (carrier-only): Works correctly (no exit_phi_inputs)
- Pattern 2 (expr result): Design complete, SSA-undef fix deferred to Phase 33-15

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

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
nyash-codex
2025-12-07 05:07:28 +09:00
parent 35f5a48eb0
commit a09ce0cbff
11 changed files with 339 additions and 70 deletions

View File

@ -34,12 +34,17 @@
//! 3. Maintain backward compatibility (no breaking changes) //! 3. Maintain backward compatibility (no breaking changes)
//! 4. Enable independent testing and evolution //! 4. Enable independent testing and evolution
//! //!
//! ## Phase 33-13: Carrier PHI Integration
//! The ExitLineOrchestrator now receives carrier_phis from exit_phi_builder
//! and uses them in ExitLineReconnector instead of the remapped exit values.
//! This ensures variable_map points to PHI-defined values (SSA-correct).
//!
//! ## Future Extensions //! ## Future Extensions
//! When implementing Pattern 4 (continue), new pattern lowerers can: //! When implementing Pattern 4 (continue), new pattern lowerers can:
//! ```rust //! ```rust
//! let exit_bindings = ExitMetaCollector::collect(self, &exit_meta, debug); //! let exit_bindings = ExitMetaCollector::collect(self, &exit_meta, debug);
//! let boundary = JoinInlineBoundary::new_with_exits(...); //! let boundary = JoinInlineBoundary::new_with_exits(...);
//! exit_line::ExitLineOrchestrator::execute(builder, &boundary, &remapper, debug); //! exit_line::ExitLineOrchestrator::execute(builder, &boundary, &carrier_phis, debug);
//! ``` //! ```
//! No changes to exit_line module needed! //! No changes to exit_line module needed!
@ -49,6 +54,9 @@ pub mod meta_collector;
pub use reconnector::ExitLineReconnector; pub use reconnector::ExitLineReconnector;
pub use meta_collector::ExitMetaCollector; pub use meta_collector::ExitMetaCollector;
use std::collections::BTreeMap;
use crate::mir::ValueId;
/// Phase 33-10-Refactor-P2: ExitLineOrchestrator facade /// Phase 33-10-Refactor-P2: ExitLineOrchestrator facade
/// ///
/// Coordinates the entire exit line reconnection process for Phase 6. /// Coordinates the entire exit line reconnection process for Phase 6.
@ -61,7 +69,7 @@ impl ExitLineOrchestrator {
/// # Inputs /// # Inputs
/// - builder: MirBuilder with variable_map to update /// - builder: MirBuilder with variable_map to update
/// - boundary: JoinInlineBoundary with exit_bindings /// - boundary: JoinInlineBoundary with exit_bindings
/// - remapper: JoinIrIdRemapper for ValueId lookup /// - carrier_phis: Map from carrier name to PHI dst ValueId (Phase 33-13)
/// - debug: Debug logging enabled /// - debug: Debug logging enabled
/// ///
/// # Returns /// # Returns
@ -69,21 +77,22 @@ impl ExitLineOrchestrator {
/// ///
/// # Process /// # Process
/// 1. Validate exit_bindings (empty case) /// 1. Validate exit_bindings (empty case)
/// 2. Delegate to ExitLineReconnector /// 2. Delegate to ExitLineReconnector with carrier_phis
pub fn execute( pub fn execute(
builder: &mut crate::mir::builder::MirBuilder, builder: &mut crate::mir::builder::MirBuilder,
boundary: &crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary, boundary: &crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary,
remapper: &crate::mir::builder::joinir_id_remapper::JoinIrIdRemapper, carrier_phis: &BTreeMap<String, ValueId>,
debug: bool, debug: bool,
) -> Result<(), String> { ) -> Result<(), String> {
if debug { if debug {
eprintln!( eprintln!(
"[cf_loop/joinir/exit_line] ExitLineOrchestrator: Starting Phase 6 reconnection" "[cf_loop/joinir/exit_line] ExitLineOrchestrator: Starting Phase 6 reconnection with {} carrier PHIs",
carrier_phis.len()
); );
} }
// Delegate to ExitLineReconnector // Phase 33-13: Delegate to ExitLineReconnector with carrier_phis
ExitLineReconnector::reconnect(builder, boundary, remapper, debug)?; ExitLineReconnector::reconnect(builder, boundary, carrier_phis, debug)?;
if debug { if debug {
eprintln!("[cf_loop/joinir/exit_line] ExitLineOrchestrator: Phase 6 complete"); eprintln!("[cf_loop/joinir/exit_line] ExitLineOrchestrator: Phase 6 complete");

View File

@ -3,32 +3,43 @@
//! Modularizes the exit line reconnection logic (Phase 6 from merge/mod.rs) //! Modularizes the exit line reconnection logic (Phase 6 from merge/mod.rs)
//! into a focused, testable Box. //! into a focused, testable Box.
//! //!
//! **Responsibility**: Update host variable_map with remapped exit values from JoinIR //! **Responsibility**: Update host variable_map with PHI dst values from exit block
//!
//! # Phase 33-13 Architecture Change
//!
//! Before: Used remapper to get remapped exit values (SSA-incorrect!)
//! After: Uses carrier_phis map from exit_phi_builder (SSA-correct!)
//!
//! The remapped exit value is an INPUT to the PHI, not the OUTPUT.
//! Only the PHI dst is defined in the exit block and can be referenced later.
use crate::mir::builder::joinir_id_remapper::JoinIrIdRemapper;
use crate::mir::builder::MirBuilder; use crate::mir::builder::MirBuilder;
use crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary; use crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary;
use crate::mir::ValueId;
use std::collections::BTreeMap;
/// ExitLineReconnector: A Box that manages exit value reconnection /// ExitLineReconnector: A Box that manages exit value reconnection
/// ///
/// # Design Notes /// # Design Notes
/// ///
/// ## Why Separate Box? /// ## Why Separate Box?
/// - Responsibility: Update host variable_map with remapped exit values /// - Responsibility: Update host variable_map with PHI dst values
/// - Input: JoinInlineBoundary.exit_bindings + JoinIrIdRemapper /// - Input: JoinInlineBoundary.exit_bindings + carrier_phis map
/// - Output: Updated MirBuilder.variable_map /// - Output: Updated MirBuilder.variable_map
/// - No side effects beyond variable_map updates /// - No side effects beyond variable_map updates
/// ///
/// ## Phase 197-B Multi-Carrier Support /// ## Phase 33-13 Carrier PHI Integration
/// This Box implements the fix for multi-carrier exit binding: /// This Box now uses carrier_phis from exit_phi_builder:
/// - Before: Single exit_phi_result applied to all carriers (buggy!) /// - Before: `variable_map[carrier] = remapper.get(join_exit)` (SSA-incorrect!)
/// - After: Each carrier gets its specific remapped exit value /// - After: `variable_map[carrier] = carrier_phis[carrier]` (SSA-correct!)
/// - Example: `sum` gets ValueId(456), `count` gets ValueId(457) ///
/// The key insight is that remapped exit values are PHI INPUTS, not OUTPUTS.
/// Only the PHI dst ValueId is defined in the exit block.
/// ///
/// ## Testing Strategy /// ## Testing Strategy
/// Can be tested independently: /// Can be tested independently:
/// 1. Create mock boundary with exit_bindings /// 1. Create mock boundary with exit_bindings
/// 2. Create mock remapper /// 2. Create mock carrier_phis map
/// 3. Call reconnect() and verify variable_map updates /// 3. Call reconnect() and verify variable_map updates
/// 4. No need to construct full merge/mod.rs machinery /// 4. No need to construct full merge/mod.rs machinery
/// ///
@ -36,36 +47,36 @@ use crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary;
/// ///
/// **Input**: /// **Input**:
/// - JoinInlineBoundary with exit_bindings (Carrier → join_exit_value mappings) /// - JoinInlineBoundary with exit_bindings (Carrier → join_exit_value mappings)
/// - JoinIrIdRemapper (JoinIR local → host function ValueId remappings) /// - carrier_phis: Map from carrier name to PHI dst ValueId
/// ///
/// **Effect**: /// **Effect**:
/// - Updates builder.variable_map entries for each carrier with remapped exit values /// - Updates builder.variable_map entries for each carrier with PHI dst values
/// ///
/// **Output**: /// **Output**:
/// - Result<(), String> (side effect on builder) /// - Result<(), String> (side effect on builder)
pub struct ExitLineReconnector; pub struct ExitLineReconnector;
impl ExitLineReconnector { impl ExitLineReconnector {
/// Reconnect exit values to host variable_map /// Reconnect exit values to host variable_map using carrier PHI dst values
/// ///
/// # Phase 197-B: Multi-carrier support /// # Phase 33-13: Carrier PHI Integration
/// ///
/// Previously, a single exit_phi_result was applied to all carriers, causing /// Previously, we used the remapper to get remapped exit values. This was
/// all carriers to get the same value (e.g., both sum and count = 5). /// SSA-incorrect because those values are PHI inputs, not outputs.
/// ///
/// Now, we use each binding's join_exit_value and the remapper to find /// Now, we use the carrier_phis map from exit_phi_builder, which contains
/// the actual exit value for each carrier. /// the actual PHI dst ValueIds that are defined in the exit block.
/// ///
/// # Algorithm /// # Algorithm
/// ///
/// For each exit_binding: /// For each exit_binding:
/// 1. Look up the remapped ValueId for binding.join_exit_value /// 1. Look up the PHI dst for this carrier in carrier_phis
/// 2. Update variable_map[binding.carrier_name] with remapped value /// 2. Update variable_map[binding.carrier_name] with PHI dst
/// 3. Log each update (if debug enabled) /// 3. Log each update (if debug enabled)
pub fn reconnect( pub fn reconnect(
builder: &mut MirBuilder, builder: &mut MirBuilder,
boundary: &JoinInlineBoundary, boundary: &JoinInlineBoundary,
remapper: &JoinIrIdRemapper, carrier_phis: &BTreeMap<String, ValueId>,
debug: bool, debug: bool,
) -> Result<(), String> { ) -> Result<(), String> {
// Early return for empty exit_bindings // Early return for empty exit_bindings
@ -78,33 +89,34 @@ impl ExitLineReconnector {
if debug { if debug {
eprintln!( eprintln!(
"[cf_loop/joinir/exit_line] ExitLineReconnector: Reconnecting {} exit bindings", "[cf_loop/joinir/exit_line] ExitLineReconnector: Reconnecting {} exit bindings with {} carrier PHIs",
boundary.exit_bindings.len() boundary.exit_bindings.len(),
carrier_phis.len()
); );
} }
// Process each exit binding // Process each exit binding
for binding in &boundary.exit_bindings { for binding in &boundary.exit_bindings {
// Look up the remapped exit value // Phase 33-13: Look up the PHI dst for this carrier
let remapped_exit = remapper.get_value(binding.join_exit_value); let phi_dst = carrier_phis.get(&binding.carrier_name);
if debug { if debug {
eprintln!( eprintln!(
"[cf_loop/joinir/exit_line] ExitLineReconnector: Carrier '{}' join_exit={:?} → remapped={:?}", "[cf_loop/joinir/exit_line] ExitLineReconnector: Carrier '{}' → phi_dst={:?}",
binding.carrier_name, binding.join_exit_value, remapped_exit binding.carrier_name, phi_dst
); );
} }
// Update variable_map if remapping found // Update variable_map with PHI dst
if let Some(remapped_value) = remapped_exit { if let Some(&phi_value) = phi_dst {
if let Some(var_vid) = builder.variable_map.get_mut(&binding.carrier_name) { if let Some(var_vid) = builder.variable_map.get_mut(&binding.carrier_name) {
if debug { if debug {
eprintln!( eprintln!(
"[cf_loop/joinir/exit_line] ExitLineReconnector: Updated variable_map['{}'] {:?}{:?}", "[cf_loop/joinir/exit_line] ExitLineReconnector: Updated variable_map['{}'] {:?}{:?} (PHI dst)",
binding.carrier_name, var_vid, remapped_value binding.carrier_name, var_vid, phi_value
); );
} }
*var_vid = remapped_value; *var_vid = phi_value;
} else if debug { } else if debug {
eprintln!( eprintln!(
"[cf_loop/joinir/exit_line] ExitLineReconnector WARNING: Carrier '{}' not found in variable_map", "[cf_loop/joinir/exit_line] ExitLineReconnector WARNING: Carrier '{}' not found in variable_map",
@ -113,8 +125,8 @@ impl ExitLineReconnector {
} }
} else if debug { } else if debug {
eprintln!( eprintln!(
"[cf_loop/joinir/exit_line] ExitLineReconnector WARNING: No remapped value for join_exit={:?}", "[cf_loop/joinir/exit_line] ExitLineReconnector WARNING: No PHI dst for carrier '{}' (may be condition-only variable)",
binding.join_exit_value binding.carrier_name
); );
} }
} }

View File

@ -1,22 +1,31 @@
//! JoinIR Exit PHI Builder //! JoinIR Exit PHI Builder
//! //!
//! Constructs the exit block PHI node that merges return values //! Constructs the exit block PHI nodes that merge return values
//! from all inlined JoinIR functions. //! from all inlined JoinIR functions.
//! //!
//! Phase 4 Extraction: Separated from merge_joinir_mir_blocks (lines 581-615) //! Phase 4 Extraction: Separated from merge_joinir_mir_blocks (lines 581-615)
//! Phase 33-13: Extended to support carrier PHIs for multi-carrier loops
use crate::mir::{BasicBlock, BasicBlockId, MirInstruction, ValueId}; use crate::mir::{BasicBlock, BasicBlockId, MirInstruction, ValueId};
use std::collections::BTreeMap;
/// Phase 5: Create exit block with PHI for return values from JoinIR functions /// Phase 5: Create exit block with PHI for return values and carrier values
/// ///
/// Phase 189-Fix: Generate exit PHI if there are multiple return values. /// Phase 189-Fix: Generate exit PHI if there are multiple return values.
/// If no return values, creates empty exit block and returns None. /// Phase 33-13: Also generates PHI for each carrier variable.
///
/// Returns:
/// - Option<ValueId>: The expr result PHI dst (if any return values)
/// - BTreeMap<String, ValueId>: Carrier name → PHI dst mapping
pub(super) fn build_exit_phi( pub(super) fn build_exit_phi(
builder: &mut crate::mir::builder::MirBuilder, builder: &mut crate::mir::builder::MirBuilder,
exit_block_id: BasicBlockId, exit_block_id: BasicBlockId,
exit_phi_inputs: &[(BasicBlockId, ValueId)], exit_phi_inputs: &[(BasicBlockId, ValueId)],
carrier_inputs: &BTreeMap<String, Vec<(BasicBlockId, ValueId)>>,
debug: bool, debug: bool,
) -> Result<Option<ValueId>, String> { ) -> Result<(Option<ValueId>, BTreeMap<String, ValueId>), String> {
let mut carrier_phis: BTreeMap<String, ValueId> = BTreeMap::new();
let exit_phi_result_id = if let Some(ref mut func) = builder.current_function { let exit_phi_result_id = if let Some(ref mut func) = builder.current_function {
let mut exit_block = BasicBlock::new(exit_block_id); let mut exit_block = BasicBlock::new(exit_block_id);
@ -35,7 +44,7 @@ pub(super) fn build_exit_phi(
.push(crate::ast::Span::unknown()); .push(crate::ast::Span::unknown());
if debug { if debug {
eprintln!( eprintln!(
"[cf_loop/joinir] Exit block PHI: {:?} = phi {:?}", "[cf_loop/joinir] Exit block PHI (expr result): {:?} = phi {:?}",
phi_dst, exit_phi_inputs phi_dst, exit_phi_inputs
); );
} }
@ -44,11 +53,40 @@ pub(super) fn build_exit_phi(
None None
}; };
// Phase 33-13: Create PHI for each carrier variable
// This ensures that carrier exit values are properly merged when
// there are multiple paths to the exit block
for (carrier_name, inputs) in carrier_inputs {
if inputs.is_empty() {
continue;
}
// Allocate a new ValueId for this carrier's PHI
let phi_dst = builder.value_gen.next();
exit_block.instructions.push(MirInstruction::Phi {
dst: phi_dst,
inputs: inputs.clone(),
type_hint: None,
});
exit_block
.instruction_spans
.push(crate::ast::Span::unknown());
carrier_phis.insert(carrier_name.clone(), phi_dst);
if debug {
eprintln!(
"[cf_loop/joinir] Exit block PHI (carrier '{}'): {:?} = phi {:?}",
carrier_name, phi_dst, inputs
);
}
}
func.add_block(exit_block); func.add_block(exit_block);
if debug { if debug {
eprintln!( eprintln!(
"[cf_loop/joinir] Created exit block: {:?}", "[cf_loop/joinir] Created exit block: {:?} with {} carrier PHIs",
exit_block_id exit_block_id, carrier_phis.len()
); );
} }
phi_result phi_result
@ -56,5 +94,5 @@ pub(super) fn build_exit_phi(
None None
}; };
Ok(exit_phi_result_id) Ok((exit_phi_result_id, carrier_phis))
} }

View File

@ -10,11 +10,22 @@ use crate::mir::builder::joinir_id_remapper::JoinIrIdRemapper;
use crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary; use crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary;
use std::collections::HashMap; use std::collections::HashMap;
/// Phase 33-13: Return type for merge_and_rewrite
///
/// Contains all information needed for exit PHI construction.
pub struct MergeResult {
/// The ID of the exit block where all Returns jump to
pub exit_block_id: BasicBlockId,
/// Vec of (from_block, return_value) for expr result PHI generation
pub exit_phi_inputs: Vec<(BasicBlockId, ValueId)>,
/// Map of carrier_name → Vec of (from_block, exit_value) for carrier PHI generation
pub carrier_inputs: std::collections::BTreeMap<String, Vec<(BasicBlockId, ValueId)>>,
}
/// Phase 4: Merge ALL functions and rewrite instructions /// Phase 4: Merge ALL functions and rewrite instructions
/// ///
/// Returns: /// Returns:
/// - exit_block_id: The ID of the exit block where all Returns jump to /// - MergeResult containing exit_block_id, exit_phi_inputs, and carrier_inputs
/// - exit_phi_inputs: Vec of (from_block, return_value) for exit PHI generation
pub(super) fn merge_and_rewrite( pub(super) fn merge_and_rewrite(
builder: &mut crate::mir::builder::MirBuilder, builder: &mut crate::mir::builder::MirBuilder,
mir_module: &MirModule, mir_module: &MirModule,
@ -23,7 +34,7 @@ pub(super) fn merge_and_rewrite(
function_params: &HashMap<String, Vec<ValueId>>, function_params: &HashMap<String, Vec<ValueId>>,
boundary: Option<&JoinInlineBoundary>, boundary: Option<&JoinInlineBoundary>,
debug: bool, debug: bool,
) -> Result<(BasicBlockId, Vec<(BasicBlockId, ValueId)>), String> { ) -> Result<MergeResult, String> {
// Create exit block for Return conversion (single for all functions) // Create exit block for Return conversion (single for all functions)
let exit_block_id = builder.block_gen.next(); let exit_block_id = builder.block_gen.next();
if debug { if debug {
@ -38,6 +49,11 @@ pub(super) fn merge_and_rewrite(
// Phase 189-Fix: Collect return values from JoinIR functions for exit PHI // Phase 189-Fix: Collect return values from JoinIR functions for exit PHI
let mut exit_phi_inputs: Vec<(BasicBlockId, ValueId)> = Vec::new(); let mut exit_phi_inputs: Vec<(BasicBlockId, ValueId)> = Vec::new();
// Phase 33-13: Collect carrier exit values for carrier PHIs
// Map from carrier_name to Vec of (from_block, exit_value)
let mut carrier_inputs: std::collections::BTreeMap<String, Vec<(BasicBlockId, ValueId)>> =
std::collections::BTreeMap::new();
// Build function_entry_map for Call→Jump conversion // Build function_entry_map for Call→Jump conversion
let mut function_entry_map: HashMap<String, BasicBlockId> = HashMap::new(); let mut function_entry_map: HashMap<String, BasicBlockId> = HashMap::new();
for (func_name, func) in &mir_module.functions { for (func_name, func) in &mir_module.functions {
@ -314,6 +330,7 @@ pub(super) fn merge_and_rewrite(
// Convert Return to Jump to exit block // Convert Return to Jump to exit block
// All functions return to same exit block (Phase 189) // All functions return to same exit block (Phase 189)
// Phase 189-Fix: Add Copy instruction to pass return value to exit PHI // 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 { if let Some(ret_val) = value {
let remapped_val = remapper.get_value(*ret_val).unwrap_or(*ret_val); let remapped_val = remapper.get_value(*ret_val).unwrap_or(*ret_val);
if debug { if debug {
@ -322,9 +339,38 @@ pub(super) fn merge_and_rewrite(
remapped_val remapped_val
); );
} }
// Collect (from_block, return_value) for exit PHI generation // Phase 33-14: Only collect for exit PHI if this loop has an expr result
exit_phi_inputs.push((new_block_id, remapped_val)); // 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-13: Collect carrier exit values for carrier PHIs
// When jumping to exit_block, collect each carrier's exit 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())
.or_insert_with(Vec::new)
.push((new_block_id, remapped_exit));
if debug {
eprintln!(
"[cf_loop/joinir] Phase 33-13: Carrier '{}' exit: ({:?}, {:?})",
binding.carrier_name, new_block_id, remapped_exit
);
}
}
}
}
MirInstruction::Jump { MirInstruction::Jump {
target: exit_block_id, target: exit_block_id,
} }
@ -429,5 +475,9 @@ pub(super) fn merge_and_rewrite(
} }
} }
Ok((exit_block_id, exit_phi_inputs)) Ok(MergeResult {
exit_block_id,
exit_phi_inputs,
carrier_inputs,
})
} }

View File

@ -106,7 +106,7 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
remap_values(builder, &used_values, &mut remapper, debug)?; remap_values(builder, &used_values, &mut remapper, debug)?;
// Phase 4: Merge blocks and rewrite instructions // Phase 4: Merge blocks and rewrite instructions
let (exit_block_id, exit_phi_inputs) = instruction_rewriter::merge_and_rewrite( let merge_result = instruction_rewriter::merge_and_rewrite(
builder, builder,
mir_module, mir_module,
&mut remapper, &mut remapper,
@ -116,21 +116,26 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
debug, debug,
)?; )?;
// Phase 5: Build exit PHI // Phase 5: Build exit PHI (expr result and carrier PHIs)
let exit_phi_result_id = exit_phi_builder::build_exit_phi( // 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(
builder, builder,
exit_block_id, merge_result.exit_block_id,
&exit_phi_inputs, &merge_result.exit_phi_inputs,
&merge_result.carrier_inputs,
debug, debug,
)?; )?;
// Phase 6: Reconnect boundary (if specified) // Phase 6: Reconnect boundary (if specified)
// Phase 197-B: Pass remapper to enable per-carrier exit value lookup // Phase 197-B: Pass remapper to enable per-carrier exit value lookup
// Phase 33-10-Refactor-P3: Delegate to ExitLineOrchestrator // Phase 33-10-Refactor-P3: Delegate to ExitLineOrchestrator
// Phase 33-13: Pass carrier_phis for proper variable_map update
if let Some(boundary) = boundary { if let Some(boundary) = boundary {
exit_line::ExitLineOrchestrator::execute(builder, boundary, &remapper, debug)?; exit_line::ExitLineOrchestrator::execute(builder, boundary, &carrier_phis, debug)?;
} }
let exit_block_id = merge_result.exit_block_id;
// Jump from current block to entry function's entry block // Jump from current block to entry function's entry block
let (entry_func_name, entry_func) = mir_module let (entry_func_name, entry_func) = mir_module
.functions .functions

View File

@ -387,6 +387,8 @@ mod tests {
join_outputs: vec![], join_outputs: vec![],
exit_bindings: vec![], // Phase 171: Add missing field exit_bindings: vec![], // Phase 171: Add missing field
condition_inputs: vec![], // Phase 171: Add missing field 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
}; };
builder.apply_to_boundary(&mut boundary) builder.apply_to_boundary(&mut boundary)

View File

@ -142,8 +142,8 @@ impl MirBuilder {
}; };
// Phase 169 / Phase 171-fix / Phase 172-3: Call Pattern 2 lowerer with ConditionEnv // Phase 169 / Phase 171-fix / Phase 172-3: Call Pattern 2 lowerer with ConditionEnv
// Now returns (JoinModule, ExitMeta) for proper exit_bindings construction // Phase 33-14: Now returns (JoinModule, JoinFragmentMeta) for expr_result + carrier separation
let (join_module, exit_meta) = match lower_loop_with_break_minimal(scope, condition, &env, &loop_var_name) { let (join_module, fragment_meta) = match lower_loop_with_break_minimal(scope, condition, &env, &loop_var_name) {
Ok((module, meta)) => (module, meta), Ok((module, meta)) => (module, meta),
Err(e) => { Err(e) => {
// Phase 195: Use unified trace // Phase 195: Use unified trace
@ -152,6 +152,9 @@ impl MirBuilder {
} }
}; };
// Phase 33-14: Extract exit_meta from fragment_meta for backward compatibility
let exit_meta = &fragment_meta.exit_meta;
// Phase 195: Use unified trace // Phase 195: Use unified trace
trace::trace().joinir_stats( trace::trace().joinir_stats(
"pattern2", "pattern2",
@ -186,12 +189,14 @@ impl MirBuilder {
let exit_bindings = ExitMetaCollector::collect(self, &exit_meta, debug); let exit_bindings = ExitMetaCollector::collect(self, &exit_meta, debug);
// Phase 172-3: Build boundary with both condition_bindings and exit_bindings // Phase 172-3: Build boundary with both condition_bindings and exit_bindings
// Phase 33-14: Set expr_result from fragment_meta
let mut boundary = crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary::new_inputs_only( let mut boundary = crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary::new_inputs_only(
vec![ValueId(0)], // JoinIR's main() parameter (loop variable init) vec![ValueId(0)], // JoinIR's main() parameter (loop variable init)
vec![loop_var_id], // Host's loop variable vec![loop_var_id], // Host's loop variable
); );
boundary.condition_bindings = condition_bindings; boundary.condition_bindings = condition_bindings;
boundary.exit_bindings = exit_bindings.clone(); boundary.exit_bindings = exit_bindings.clone();
boundary.expr_result = fragment_meta.expr_result; // Phase 33-14: Pass expr_result to merger
// Phase 189: Capture exit PHI result (now used for reconnect) // Phase 189: Capture exit PHI result (now used for reconnect)
let _ = self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?; let _ = self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?;

View File

@ -75,6 +75,7 @@ impl MirBuilder {
// Phase 170-A-1: Test methods (simplified versions) // Phase 170-A-1: Test methods (simplified versions)
"TrimTest.trim/1" => true, "TrimTest.trim/1" => true,
"Main.trim/1" => true, // Phase 171-fix: Main box variant "Main.trim/1" => true, // Phase 171-fix: Main box variant
"Main.trim_string_simple/1" => true, // Phase 33-13: Simple trim variant
_ => false, _ => false,
}; };

View File

@ -206,6 +206,115 @@ pub struct ExitMeta {
pub exit_values: Vec<(String, ValueId)>, pub exit_values: Vec<(String, ValueId)>,
} }
/// Phase 33-14: JoinFragmentMeta - Distinguishes expr result from carrier updates
///
/// ## Purpose
///
/// Separates two distinct use cases for JoinIR loops:
///
/// 1. **Expr Result Pattern** (joinir_min_loop.hako):
/// ```nyash
/// local result = loop(...) { ... } // Loop used as expression
/// return result
/// ```
/// Here, the k_exit return value is the "expr result" that should go to exit_phi_inputs.
///
/// 2. **Carrier Update Pattern** (trim pattern):
/// ```nyash
/// loop(...) { start = start + 1 } // Loop used for side effects
/// print(start) // Use carrier after loop
/// ```
/// Here, there's no "expr result" - only carrier variable updates.
///
/// ## SSA Correctness
///
/// Previously, exit_phi_inputs mixed expr results with carrier updates, causing:
/// - PHI inputs that referenced undefined remapped values
/// - SSA-undef errors in VM execution
///
/// With JoinFragmentMeta:
/// - `expr_result`: Only goes to exit_phi_inputs (generates PHI for expr value)
/// - `exit_meta`: Only goes to carrier_inputs (updates variable_map via carrier PHIs)
///
/// ## Example: Pattern 2 (joinir_min_loop.hako)
///
/// ```rust
/// JoinFragmentMeta {
/// expr_result: Some(i_exit), // k_exit returns i as expr value
/// exit_meta: ExitMeta::single("i".to_string(), i_exit), // Also a carrier
/// }
/// ```
///
/// ## Example: Pattern 3 (trim pattern)
///
/// ```rust
/// JoinFragmentMeta {
/// expr_result: None, // Loop doesn't return a value
/// exit_meta: ExitMeta::multiple(vec![
/// ("start".to_string(), start_exit),
/// ("end".to_string(), end_exit),
/// ]),
/// }
/// ```
#[derive(Debug, Clone)]
pub struct JoinFragmentMeta {
/// Expression result ValueId from k_exit (JoinIR-local)
///
/// - `Some(vid)`: Loop is used as expression, k_exit's return value → exit_phi_inputs
/// - `None`: Loop is used for side effects only, no PHI for expr value
pub expr_result: Option<ValueId>,
/// Carrier variable exit bindings (existing ExitMeta)
///
/// Maps carrier names to their JoinIR-local exit values.
/// These go to carrier_inputs for carrier PHI generation.
pub exit_meta: ExitMeta,
}
impl JoinFragmentMeta {
/// Create JoinFragmentMeta for expression result pattern
///
/// Use when the loop returns a value (like `return loop(...)`).
pub fn with_expr_result(expr_result: ValueId, exit_meta: ExitMeta) -> Self {
Self {
expr_result: Some(expr_result),
exit_meta,
}
}
/// Create JoinFragmentMeta for carrier-only pattern
///
/// Use when the loop only updates carriers (like trim pattern).
pub fn carrier_only(exit_meta: ExitMeta) -> Self {
Self {
expr_result: None,
exit_meta,
}
}
/// Create empty JoinFragmentMeta (no expr result, no carriers)
pub fn empty() -> Self {
Self {
expr_result: None,
exit_meta: ExitMeta::empty(),
}
}
/// Check if this fragment has an expression result
pub fn has_expr_result(&self) -> bool {
self.expr_result.is_some()
}
/// Phase 33-14: Backward compatibility - convert to ExitMeta
///
/// During migration, some code may still expect ExitMeta.
/// This extracts just the carrier bindings.
#[deprecated(since = "33-14", note = "Use exit_meta directly for carrier access")]
pub fn to_exit_meta(&self) -> ExitMeta {
self.exit_meta.clone()
}
}
impl ExitMeta { impl ExitMeta {
/// Create new ExitMeta with no exit values /// Create new ExitMeta with no exit values
pub fn empty() -> Self { pub fn empty() -> Self {

View File

@ -205,6 +205,33 @@ pub struct JoinInlineBoundary {
/// ///
/// This replaces `condition_inputs` to ensure proper ValueId separation. /// This replaces `condition_inputs` to ensure proper ValueId separation.
pub condition_bindings: Vec<super::condition_to_joinir::ConditionBinding>, pub condition_bindings: Vec<super::condition_to_joinir::ConditionBinding>,
/// Phase 33-14: Expression result ValueId (JoinIR-local)
///
/// If the loop is used as an expression (like `return loop(...)`), this field
/// contains the JoinIR-local ValueId of k_exit's return value.
///
/// - `Some(ValueId)`: Loop returns a value → k_exit return goes to exit_phi_inputs
/// - `None`: Loop only updates carriers → no exit_phi_inputs generation
///
/// # Example: joinir_min_loop.hako (expr result pattern)
///
/// ```nyash
/// loop(i < 3) { if (i >= 2) { break } i = i + 1 }
/// return i
/// ```
///
/// Here, `expr_result = Some(i_exit)` because the loop's result is used.
///
/// # Example: trim pattern (carrier-only)
///
/// ```nyash
/// loop(start < end) { start = start + 1 }
/// print(start) // Uses carrier after loop
/// ```
///
/// Here, `expr_result = None` because the loop doesn't return a value.
pub expr_result: Option<crate::mir::ValueId>,
} }
impl JoinInlineBoundary { impl JoinInlineBoundary {
@ -229,6 +256,7 @@ impl JoinInlineBoundary {
#[allow(deprecated)] #[allow(deprecated)]
condition_inputs: vec![], // Phase 171: Default to empty (deprecated) condition_inputs: vec![], // Phase 171: Default to empty (deprecated)
condition_bindings: vec![], // Phase 171-fix: Default to empty condition_bindings: vec![], // Phase 171-fix: Default to empty
expr_result: None, // Phase 33-14: Default to carrier-only pattern
} }
} }
@ -268,6 +296,7 @@ impl JoinInlineBoundary {
#[allow(deprecated)] #[allow(deprecated)]
condition_inputs: vec![], // Phase 171: Default to empty (deprecated) condition_inputs: vec![], // Phase 171: Default to empty (deprecated)
condition_bindings: vec![], // Phase 171-fix: Default to empty condition_bindings: vec![], // Phase 171-fix: Default to empty
expr_result: None, // Phase 33-14
} }
} }
@ -303,6 +332,7 @@ impl JoinInlineBoundary {
#[allow(deprecated)] #[allow(deprecated)]
condition_inputs: vec![], // Phase 171: Default to empty (deprecated) condition_inputs: vec![], // Phase 171: Default to empty (deprecated)
condition_bindings: vec![], // Phase 171-fix: Default to empty condition_bindings: vec![], // Phase 171-fix: Default to empty
expr_result: None, // Phase 33-14
} }
} }
@ -360,6 +390,7 @@ impl JoinInlineBoundary {
#[allow(deprecated)] #[allow(deprecated)]
condition_inputs: vec![], // Phase 171: Default to empty (deprecated) condition_inputs: vec![], // Phase 171: Default to empty (deprecated)
condition_bindings: vec![], // Phase 171-fix: Default to empty condition_bindings: vec![], // Phase 171-fix: Default to empty
expr_result: None, // Phase 33-14
} }
} }
@ -403,6 +434,7 @@ impl JoinInlineBoundary {
#[allow(deprecated)] #[allow(deprecated)]
condition_inputs, condition_inputs,
condition_bindings: vec![], // Phase 171-fix: Will be populated by new constructor condition_bindings: vec![], // Phase 171-fix: Will be populated by new constructor
expr_result: None, // Phase 33-14
} }
} }
@ -450,6 +482,7 @@ impl JoinInlineBoundary {
#[allow(deprecated)] #[allow(deprecated)]
condition_inputs, condition_inputs,
condition_bindings: vec![], // Phase 171-fix: Will be populated by new constructor condition_bindings: vec![], // Phase 171-fix: Will be populated by new constructor
expr_result: None, // Phase 33-14
} }
} }
@ -504,6 +537,7 @@ impl JoinInlineBoundary {
#[allow(deprecated)] #[allow(deprecated)]
condition_inputs: vec![], // Deprecated, use condition_bindings instead condition_inputs: vec![], // Deprecated, use condition_bindings instead
condition_bindings, condition_bindings,
expr_result: None, // Phase 33-14
} }
} }
} }

View File

@ -56,7 +56,7 @@
//! Following the "80/20 rule" from CLAUDE.md - get it working first, generalize later. //! Following the "80/20 rule" from CLAUDE.md - get it working first, generalize later.
use crate::ast::ASTNode; use crate::ast::ASTNode;
use crate::mir::join_ir::lowering::carrier_info::ExitMeta; 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::condition_to_joinir::{lower_condition_to_joinir, ConditionEnv};
use crate::mir::join_ir::lowering::loop_scope_shape::LoopScopeShape; use crate::mir::join_ir::lowering::loop_scope_shape::LoopScopeShape;
use crate::mir::join_ir::{ use crate::mir::join_ir::{
@ -109,9 +109,11 @@ use crate::mir::ValueId;
/// This ensures JoinIR never accesses HOST ValueIds directly. /// This ensures JoinIR never accesses HOST ValueIds directly.
/// ///
/// # Phase 172-3: ExitMeta Return /// # Phase 172-3: ExitMeta Return
/// # Phase 33-14: JoinFragmentMeta Return
/// ///
/// Returns `(JoinModule, ExitMeta)` where ExitMeta contains the JoinIR-local ValueId /// Returns `(JoinModule, JoinFragmentMeta)` where:
/// of the k_exit parameter. This allows the caller to build proper exit_bindings. /// - `expr_result`: k_exit's return value (i_exit) - this is what `return i` uses
/// - `exit_meta`: carrier bindings for variable_map updates
/// ///
/// # Arguments /// # Arguments
/// ///
@ -121,7 +123,7 @@ pub fn lower_loop_with_break_minimal(
condition: &ASTNode, condition: &ASTNode,
env: &ConditionEnv, env: &ConditionEnv,
loop_var_name: &str, loop_var_name: &str,
) -> Result<(JoinModule, ExitMeta), String> { ) -> Result<(JoinModule, JoinFragmentMeta), String> {
// Phase 188-Impl-2: Use local ValueId allocator (sequential from 0) // Phase 188-Impl-2: Use local ValueId allocator (sequential from 0)
// JoinIR has NO knowledge of host ValueIds - boundary handled separately // JoinIR has NO knowledge of host ValueIds - boundary handled separately
let mut value_counter = 0u32; let mut value_counter = 0u32;
@ -305,9 +307,11 @@ pub fn lower_loop_with_break_minimal(
eprintln!("[joinir/pattern2] Condition from AST (not hardcoded)"); eprintln!("[joinir/pattern2] Condition from AST (not hardcoded)");
eprintln!("[joinir/pattern2] Exit PHI: k_exit receives i from both natural exit and break"); eprintln!("[joinir/pattern2] Exit PHI: k_exit receives i from both natural exit and break");
// Phase 172-3: Build ExitMeta with k_exit parameter's ValueId // Phase 172-3 → Phase 33-14: Build JoinFragmentMeta with expr_result
// Pattern 2: Loop is used as expression → `return i` means k_exit's return is expr_result
let exit_meta = ExitMeta::single(loop_var_name.to_string(), i_exit); let exit_meta = ExitMeta::single(loop_var_name.to_string(), i_exit);
eprintln!("[joinir/pattern2] Phase 172-3: ExitMeta {{ {}{:?} }}", loop_var_name, i_exit); let fragment_meta = JoinFragmentMeta::with_expr_result(i_exit, exit_meta);
eprintln!("[joinir/pattern2] Phase 33-14: JoinFragmentMeta {{ expr_result: {:?}, carrier: {} }}", i_exit, loop_var_name);
Ok((join_module, exit_meta)) Ok((join_module, fragment_meta))
} }