feat(joinir): Phase 202-C Pattern 4 uses JoinValueSpace, unify dual counters

# Summary
Pattern 4 (Continue) now uses JoinValueSpace for unified ValueId allocation,
eliminating the dual counter system that was prone to ValueId collisions.

# Changes
- loop_with_continue_minimal.rs: Replace value_counter + join_value_counter with JoinValueSpace
  - Param region (100+): ConditionEnv variables
  - Local region (1000+): All intermediate values (Const, BinOp, etc.)
  - Eliminated manual counter management (value_counter = 0u32, join_value_counter++)
- pattern4_with_continue.rs: Create JoinValueSpace and pass to lowerer
- Added Phase 202-C documentation explaining the unification

# Test Results
 cargo test --release --lib continue: 11 passed (3 ignored)
 E2E apps/tests/loop_continue_pattern4.hako → 25 (single carrier)
 E2E apps/tests/loop_continue_multi_carrier.hako → 100, 10 (multi carrier)

# Benefits
- No ValueId collision risk (disjoint regions)
- Consistent with Pattern 2 architecture (Phase 201)
- Simplified allocation logic (no manual counter tracking)
- Better maintainability (single source of truth)

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

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
nyash-codex
2025-12-09 19:20:46 +09:00
parent 98e81b2650
commit ae741d97b4
2 changed files with 54 additions and 34 deletions

View File

@ -292,8 +292,19 @@ impl MirBuilder {
} }
} }
// Phase 169: Call Pattern 4 lowerer with condition AST // Phase 202-C: Create JoinValueSpace for unified ValueId allocation
let (join_module, exit_meta) = match lower_loop_with_continue_minimal(scope, condition, self, &carrier_info, &carrier_updates) { use crate::mir::join_ir::lowering::join_value_space::JoinValueSpace;
let mut join_value_space = JoinValueSpace::new();
// Phase 169 / Phase 202-C: Call Pattern 4 lowerer with JoinValueSpace
let (join_module, exit_meta) = match lower_loop_with_continue_minimal(
scope,
condition,
self,
&carrier_info,
&carrier_updates,
&mut join_value_space,
) {
Ok(result) => result, Ok(result) => result,
Err(e) => { Err(e) => {
// Phase 195: Use unified trace // Phase 195: Use unified trace

View File

@ -2,6 +2,7 @@
//! //!
//! Phase 195: Initial minimal implementation for single-carrier (sum only) //! Phase 195: Initial minimal implementation for single-carrier (sum only)
//! Phase 196: Extended to support multiple carrier variables (sum, count, etc.) //! Phase 196: Extended to support multiple carrier variables (sum, count, etc.)
//! Phase 202-C: Unified dual counters into JoinValueSpace
//! //!
//! Target: apps/tests/loop_continue_pattern4.hako (single carrier) //! Target: apps/tests/loop_continue_pattern4.hako (single carrier)
//! apps/tests/loop_continue_multi_carrier.hako (multi carrier) //! apps/tests/loop_continue_multi_carrier.hako (multi carrier)
@ -42,12 +43,25 @@
//! - ExitMeta::multiple() instead of ExitMeta::single() //! - ExitMeta::multiple() instead of ExitMeta::single()
//! - Dynamic parameter/slot allocation based on carrier count //! - Dynamic parameter/slot allocation based on carrier count
//! //!
//! ## Phase 202-C: JoinValueSpace Unification
//!
//! Previously used dual counters:
//! - `value_counter`: JoinIR internal ValueIds (0, 1, 2, ...)
//! - `join_value_counter`: Condition-only variables (carrier_count + 1, ...)
//!
//! Now unified via JoinValueSpace:
//! - **Param region (100+)**: ConditionEnv, CarrierInfo parameters
//! - **Local region (1000+)**: Const, BinOp, intermediate values
//!
//! This prevents ValueId collisions after remapping (Phase 201 design).
//!
//! Following the "80/20 rule" from CLAUDE.md - now generalizing after working. //! Following the "80/20 rule" from CLAUDE.md - now generalizing after working.
use crate::ast::ASTNode; use crate::ast::ASTNode;
use crate::mir::builder::MirBuilder; use crate::mir::builder::MirBuilder;
use crate::mir::join_ir::lowering::carrier_info::{CarrierInfo, ExitMeta}; use crate::mir::join_ir::lowering::carrier_info::{CarrierInfo, ExitMeta};
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::join_value_space::JoinValueSpace;
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::lowering::loop_update_analyzer::{UpdateExpr, UpdateRhs}; use crate::mir::join_ir::lowering::loop_update_analyzer::{UpdateExpr, UpdateRhs};
use crate::mir::join_ir::{ use crate::mir::join_ir::{
@ -64,15 +78,16 @@ use std::collections::HashMap;
/// Lower Pattern 4 (Loop with Continue) to JoinIR /// Lower Pattern 4 (Loop with Continue) to JoinIR
/// ///
/// # Phase 195-196: Pure JoinIR Fragment Generation /// # Phase 195-196: Pure JoinIR Fragment Generation
/// # Phase 202-C: JoinValueSpace Unification
/// ///
/// This version generates JoinIR using **local ValueIds only** (0, 1, 2, ...). /// This version generates JoinIR using **JoinValueSpace-allocated ValueIds**.
/// It has NO knowledge of the host function's ValueId space. The boundary mapping /// It has NO knowledge of the host function's ValueId space. The boundary mapping
/// is handled separately via JoinInlineBoundary. /// is handled separately via JoinInlineBoundary.
/// ///
/// ## Design Philosophy /// ## Design Philosophy
/// ///
/// - **Box A**: JoinIR Frontend (doesn't know about host ValueIds) /// - **Box A**: JoinIR Frontend (doesn't know about host ValueIds)
/// - **Box B**: This function - converts to JoinIR with local IDs /// - **Box B**: This function - converts to JoinIR with JoinValueSpace IDs
/// - **Box C**: JoinInlineBoundary - stores boundary info /// - **Box C**: JoinInlineBoundary - stores boundary info
/// - **Box D**: merge_joinir_mir_blocks - injects Copy instructions /// - **Box D**: merge_joinir_mir_blocks - injects Copy instructions
/// ///
@ -85,19 +100,20 @@ use std::collections::HashMap;
/// ///
/// * `_scope` - LoopScopeShape (reserved for future generic implementation) /// * `_scope` - LoopScopeShape (reserved for future generic implementation)
/// * `condition` - Loop condition AST node (e.g., `i < end`) (Phase 169) /// * `condition` - Loop condition AST node (e.g., `i < end`) (Phase 169)
/// * `builder` - MirBuilder for variable resolution (Phase 169) /// * `_builder` - MirBuilder for variable resolution (Phase 169)
/// * `carrier_info` - Phase 196: Carrier metadata for dynamic multi-carrier support /// * `carrier_info` - Phase 196: Carrier metadata for dynamic multi-carrier support
/// * `carrier_updates` - Phase 197: Update expressions for each carrier variable /// * `carrier_updates` - Phase 197: Update expressions for each carrier variable
/// * `join_value_space` - Phase 202-C: Unified JoinIR ValueId allocator (Local region: 1000+)
/// ///
/// # Returns /// # Returns
/// ///
/// * `Ok((JoinModule, ExitMeta))` - Successfully lowered to JoinIR with exit metadata /// * `Ok((JoinModule, ExitMeta))` - Successfully lowered to JoinIR with exit metadata
/// * `Err(String)` - Pattern not matched or lowering error /// * `Err(String)` - Pattern not matched or lowering error
/// ///
/// # Boundary Contract (Phase 196 updated) /// # Boundary Contract (Phase 196 updated, Phase 202-C: JoinValueSpace)
/// ///
/// This function returns a JoinModule with: /// This function returns a JoinModule with:
/// - **Input slots**: ValueId(0) = i_init, ValueId(1..N) = carrier values /// - **Input slots**: Local region (1000+) = i_init, carrier values
/// - **Output slots**: k_exit returns all carrier values /// - **Output slots**: k_exit returns all carrier values
/// - **Exit metadata**: ExitMeta containing all carrier bindings /// - **Exit metadata**: ExitMeta containing all carrier bindings
/// - **Caller responsibility**: Create JoinInlineBoundary to map ValueIds /// - **Caller responsibility**: Create JoinInlineBoundary to map ValueIds
@ -107,6 +123,7 @@ pub(crate) fn lower_loop_with_continue_minimal(
_builder: &mut MirBuilder, _builder: &mut MirBuilder,
carrier_info: &CarrierInfo, carrier_info: &CarrierInfo,
carrier_updates: &HashMap<String, UpdateExpr>, carrier_updates: &HashMap<String, UpdateExpr>,
join_value_space: &mut JoinValueSpace,
) -> Result<(JoinModule, ExitMeta), String> { ) -> Result<(JoinModule, ExitMeta), String> {
// Phase 170-D-impl-3: Validate that loop condition only uses supported variable scopes // Phase 170-D-impl-3: Validate that loop condition only uses supported variable scopes
// LoopConditionScopeBox checks that loop conditions don't reference loop-body-local variables // LoopConditionScopeBox checks that loop conditions don't reference loop-body-local variables
@ -127,20 +144,11 @@ pub(crate) fn lower_loop_with_continue_minimal(
loop_cond_scope.var_names() loop_cond_scope.var_names()
); );
// Phase 196: Use local ValueId allocator (sequential from 0)
// JoinIR has NO knowledge of host ValueIds - boundary handled separately
let mut value_counter = 0u32;
let mut alloc_value = || {
let id = ValueId(value_counter);
value_counter += 1;
id
};
let mut join_module = JoinModule::new(); let mut join_module = JoinModule::new();
let carrier_count = carrier_info.carriers.len(); let carrier_count = carrier_info.carriers.len();
eprintln!( eprintln!(
"[joinir/pattern4] Phase 196: Generating JoinIR for {} carriers: {:?}", "[joinir/pattern4] Phase 202-C: Generating JoinIR for {} carriers: {:?}",
carrier_count, carrier_count,
carrier_info.carriers.iter().map(|c| &c.name).collect::<Vec<_>>() carrier_info.carriers.iter().map(|c| &c.name).collect::<Vec<_>>()
); );
@ -153,43 +161,43 @@ pub(crate) fn lower_loop_with_continue_minimal(
let k_exit_id = JoinFuncId::new(2); let k_exit_id = JoinFuncId::new(2);
// ================================================================== // ==================================================================
// ValueId allocation (Phase 196: Dynamic based on carrier count) // ValueId allocation (Phase 202-C: Dynamic based on carrier count)
// ================================================================== // ==================================================================
// main() parameters: [i_init, carrier1_init, carrier2_init, ...] // main() parameters: [i_init, carrier1_init, carrier2_init, ...]
let i_init = alloc_value(); // ValueId(0) let i_init = join_value_space.alloc_local();
let mut carrier_init_ids: Vec<ValueId> = Vec::new(); let mut carrier_init_ids: Vec<ValueId> = Vec::new();
for _ in 0..carrier_count { for _ in 0..carrier_count {
carrier_init_ids.push(alloc_value()); carrier_init_ids.push(join_value_space.alloc_local());
} }
let loop_result = alloc_value(); // result from loop_step let loop_result = join_value_space.alloc_local(); // result from loop_step
// loop_step() parameters: [i_param, carrier1_param, carrier2_param, ...] // loop_step() parameters: [i_param, carrier1_param, carrier2_param, ...]
let i_param = alloc_value(); let i_param = join_value_space.alloc_local();
let mut carrier_param_ids: Vec<ValueId> = Vec::new(); let mut carrier_param_ids: Vec<ValueId> = Vec::new();
for _ in 0..carrier_count { for _ in 0..carrier_count {
carrier_param_ids.push(alloc_value()); carrier_param_ids.push(join_value_space.alloc_local());
} }
// Phase 171-fix: Build ConditionEnv for condition lowering // Phase 202-C: Build ConditionEnv for condition lowering using JoinValueSpace
// TODO(Phase 171-fix): This is a temporary workaround. Pattern 3 lowerer should build // Loop variable and condition-only variables use Param region (100+)
// ConditionEnv at the call site and pass it here, similar to Pattern 2.
// For now, we build it internally since this function still needs builder for carrier_info.
use crate::mir::join_ir::lowering::condition_to_joinir::extract_condition_variables; use crate::mir::join_ir::lowering::condition_to_joinir::extract_condition_variables;
let mut env = ConditionEnv::new(); let mut env = ConditionEnv::new();
// Add loop parameter to env (ValueId(0) in JoinIR space) // Add loop parameter to env (using i_param which is in Local region)
let loop_var_name = carrier_info.loop_var_name.clone(); let loop_var_name = carrier_info.loop_var_name.clone();
env.insert(loop_var_name.clone(), ValueId(0)); // i_param is ValueId(0) equivalent env.insert(loop_var_name.clone(), i_param);
// Extract and add condition-only variables // Extract and add condition-only variables (Param region)
let condition_var_names = extract_condition_variables(condition, &[loop_var_name.clone()]); let condition_var_names = extract_condition_variables(condition, &[loop_var_name.clone()]);
let mut join_value_counter = carrier_count as u32 + 1; // After loop param and carriers
for var_name in &condition_var_names { for var_name in &condition_var_names {
let join_id = ValueId(join_value_counter); let join_id = join_value_space.alloc_param();
join_value_counter += 1;
env.insert(var_name.clone(), join_id); env.insert(var_name.clone(), join_id);
} }
// Phase 202-C: Create allocator closure AFTER all direct allocations
// This avoids borrow checker issues
let mut alloc_value = || join_value_space.alloc_local();
// Phase 169 / Phase 171-fix: Lower condition using condition_to_joinir helper with ConditionEnv // Phase 169 / Phase 171-fix: Lower condition using condition_to_joinir helper with ConditionEnv
let (cond_value, mut cond_instructions) = lower_condition_to_joinir( let (cond_value, mut cond_instructions) = lower_condition_to_joinir(
condition, condition,
@ -502,9 +510,10 @@ pub(crate) fn lower_loop_with_continue_minimal(
// Set entry point // Set entry point
join_module.entry = Some(main_id); join_module.entry = Some(main_id);
eprintln!("[joinir/pattern4] Generated JoinIR for Loop with Continue Pattern"); eprintln!("[joinir/pattern4] Phase 202-C: Generated JoinIR for Loop with Continue Pattern");
eprintln!("[joinir/pattern4] Functions: main, loop_step, k_exit"); eprintln!("[joinir/pattern4] Functions: main, loop_step, k_exit");
eprintln!("[joinir/pattern4] Continue: Select-based skip"); eprintln!("[joinir/pattern4] Continue: Select-based skip");
eprintln!("[joinir/pattern4] ValueId allocation: JoinValueSpace (Local region 1000+)");
eprintln!( eprintln!(
"[joinir/pattern4] Carriers: {} ({:?})", "[joinir/pattern4] Carriers: {} ({:?})",
carrier_count, carrier_count,