diff --git a/docs/development/current/main/phase220-condition-env-integration.md b/docs/development/current/main/phase220-condition-env-integration.md new file mode 100644 index 00000000..39db4c99 --- /dev/null +++ b/docs/development/current/main/phase220-condition-env-integration.md @@ -0,0 +1,135 @@ +# Phase 220-B: ConditionEnv Integration for If-Sum + +## Overview +Integrated ConditionEnv infrastructure into if-sum lowerer to support +variable conditions like `loop(i < len)`. + +## Implementation Status + +### Completed +- ✅ ConditionEnv construction in `lower_if_sum_pattern()` +- ✅ `extract_value_or_variable()` function (supports both literals and variables) +- ✅ Updated `extract_loop_condition()` and `extract_if_condition()` to use ConditionEnv +- ✅ JoinIR generation includes condition-only variables as parameters +- ✅ Condition bindings wired to boundary builder +- ✅ Updated call sites in `pattern3_with_if_phi.rs` + +### In Progress +- 🔧 **Issue**: phase212_if_sum_min.hako returns RC=0 instead of RC=2 +- 🔧 **Root Cause**: Condition variable (`len`) remapping issue during merge + - HOST ValueId(6) → JoinIR ValueId(101) → After merge ValueId(10) ❌ + - Expected: HOST ValueId(6) → JoinIR ValueId(101) → After merge ValueId(6) ✅ +- 🔧 **Next Step**: Investigate condition_bindings handling in merge pipeline + +## Implementation + +### Pattern 2 Reference +Followed proven Pattern 2 (break condition) integration pattern: +1. Build ConditionEnv early with extract_condition_variables +2. Pass to condition lowering functions +3. Lookup variables via cond_env.get() +4. Wire condition_bindings to JoinInlineBoundary + +### Key Changes + +**File**: `src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs` +- Added ConditionEnv construction (lines 70-127) +- Updated signature to accept `variable_map`, `loop_var_name`, `loop_var_id` +- Return type changed to include `Vec` +- JoinIR generation updated to include condition variables as parameters + +**File**: `src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs` +- Updated call site to pass variable_map and loop context +- Wired condition_bindings to boundary builder (line 189) + +### ValueOrLiteral Enum +```rust +enum ValueOrLiteral { + Literal(i64), + Variable(String, ValueId), +} +``` + +### Function Signatures Updated +```rust +fn extract_value_or_variable( + node: &ASTNode, + cond_env: &ConditionEnv, +) -> Result + +fn extract_loop_condition( + cond: &ASTNode, + cond_env: &ConditionEnv, +) -> Result<(String, CompareOp, ValueOrLiteral), String> +``` + +## Test Results (Preliminary) + +### phase212_if_sum_min.hako +- Expected: RC=2 +- Actual: RC=0 ❌ +- Issue: Loop not executing (no print output) +- Debug: Condition variable `len` extracted correctly but remapping issue + +### Debug Trace +``` +[joinir/pattern3/if-sum] Extracted 1 condition variables: ["len"] +[joinir/pattern3/if-sum] Condition variable 'len': host=ValueId(6), join=ValueId(101) +[if-sum/joinir] loop_step params: 4 total (3 carriers + 1 condition vars) +[DEBUG-177] 'len': JoinIR ValueId(101) → Some(ValueId(10)) ← Should be ValueId(6) +``` + +## Design Principles +1. **Reuse Existing Boxes** (ConditionEnv/ConditionBinding) +2. **Follow Pattern 2 Structure** (proven blueprint) +3. **Fail-Fast** (variable not in ConditionEnv → error) +4. **ParamRole::Condition Routing** (separate from carriers) + +## Next Steps + +1. **Investigate Merge Pipeline**: + - Check how condition_bindings are processed in JoinIRConversionPipeline + - Verify that condition variables are loaded at loop entry + - Ensure remapping uses host_value from ConditionBinding + +2. **Add Load Instructions**: + - May need to generate Load instructions for condition variables at loop entry + - Similar to how Pattern 2 handles break condition variables + +3. **Test Additional Cases**: + - phase218_json_if_sum_min.hako (expect RC=10) + - Regression: loop_if_phi.hako (expect RC=2) + - Regression: phase217_if_sum_multi_min.hako (expect RC=2) + +## Files Modified + +**Primary**: +- `src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs` (ConditionEnv integration) +- `src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs` (call site update) + +**Documentation**: +- `docs/development/current/main/phase220-condition-env-integration.md` (this file) + +## Important Notes + +- **Fail-Fast Maintained**: Variables not in ConditionEnv cause explicit errors +- **Method Call Limitation**: `defs.len()` style not supported yet (Phase 221+) +- **Pattern 2 Parity**: Uses identical ConditionEnvBuilder API +- **ParamRole Separation**: Condition variables kept separate from carriers + +## Known Issues + +1. **Condition Variable Remapping** (Critical): + - Condition variables get remapped to wrong ValueIds during merge + - Need to verify condition_bindings handling in merge pipeline + +2. **No Test Passing Yet**: + - phase212: RC=0 (expected 2) ❌ + - Other tests not yet verified + +## References + +- [Phase 171-fix: ConditionEnv](../../../reference/joinir/condition-env.md) +- [Phase 200-A/B: ConditionBinding](../../../reference/joinir/condition-binding.md) +- [Phase 201: JoinValueSpace](../../../reference/joinir/join-value-space.md) +- [Pattern 2: Break Condition](./pattern2-break-condition.md) diff --git a/src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs b/src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs index ddaa7ff8..effba9e3 100644 --- a/src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs +++ b/src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs @@ -112,12 +112,15 @@ impl MirBuilder { "[cf_loop/pattern3] if-sum pattern detected but no if statement found".to_string() })?; - // Call AST-based if-sum lowerer - let (join_module, fragment_meta) = lower_if_sum_pattern( + // Phase 220-B: Call AST-based if-sum lowerer with ConditionEnv support + let (join_module, fragment_meta, cond_bindings) = 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; @@ -173,11 +176,17 @@ impl MirBuilder { ) ); - // Phase 215-2: Pass expr_result to boundary + // Phase 220-B: Wire condition_bindings to boundary builder + trace::trace().debug( + "pattern3/if-sum", + &format!("Wiring {} condition bindings to boundary", cond_bindings.len()) + ); + 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_loop_var_name(Some(ctx.loop_var_name.clone())) + .with_condition_bindings(cond_bindings); // Phase 220-B: Add condition bindings // Add expr_result if present if let Some(expr_id) = fragment_meta.expr_result { diff --git a/src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs b/src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs index 890a5151..16a3c235 100644 --- a/src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs +++ b/src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs @@ -30,13 +30,17 @@ use crate::ast::ASTNode; use crate::mir::join_ir::lowering::carrier_info::{ExitMeta, JoinFragmentMeta}; +use crate::mir::join_ir::lowering::condition_env::{ConditionBinding, ConditionEnv}; +use crate::mir::join_ir::lowering::condition_to_joinir::extract_condition_variables; use crate::mir::join_ir::lowering::join_value_space::JoinValueSpace; use crate::mir::join_ir::{ BinOpKind, CompareOp, ConstValue, JoinFuncId, JoinFunction, JoinInst, JoinModule, MirLikeInst, UnaryOp, }; +use crate::mir::ValueId; +use std::collections::BTreeMap; -/// Phase 213: Lower if-sum pattern to JoinIR using AST +/// Phase 220-B: Lower if-sum pattern to JoinIR using AST (with ConditionEnv support) /// /// # Arguments /// @@ -44,26 +48,95 @@ use crate::mir::join_ir::{ /// * `if_stmt` - If statement AST from loop body /// * `body` - Full loop body AST (for finding counter update) /// * `join_value_space` - Unified ValueId allocator +/// * `variable_map` - HOST function's variable_map for ConditionEnv construction +/// * `loop_var_name` - Loop variable name (e.g., "i") +/// * `loop_var_id` - HOST ValueId for loop variable /// /// # Returns /// -/// * `Ok((JoinModule, JoinFragmentMeta))` - JoinIR module with exit metadata +/// * `Ok((JoinModule, JoinFragmentMeta, Vec))` - JoinIR module, exit metadata, and condition bindings /// * `Err(String)` - Pattern not supported or extraction failed pub fn lower_if_sum_pattern( loop_condition: &ASTNode, if_stmt: &ASTNode, body: &[ASTNode], join_value_space: &mut JoinValueSpace, -) -> Result<(JoinModule, JoinFragmentMeta), String> { + variable_map: &BTreeMap, + loop_var_name: &str, + loop_var_id: ValueId, +) -> Result<(JoinModule, JoinFragmentMeta, Vec), String> { eprintln!("[joinir/pattern3/if-sum] Starting AST-based if-sum lowering"); - // Step 1: Extract loop condition info (e.g., i < len → var="i", op=Lt, limit=len) - let (loop_var, loop_op, loop_limit) = extract_loop_condition(loop_condition)?; - eprintln!("[joinir/pattern3/if-sum] Loop condition: {} {:?} {}", loop_var, loop_op, loop_limit); + // Phase 220-B Step 1: Build ConditionEnv (following Pattern 2 style) + let cond_vars = extract_condition_variables( + loop_condition, + &[loop_var_name.to_string()], // Exclude loop variable itself + ); - // Step 2: Extract if condition info (e.g., i > 0 → var="i", op=Gt, value=0) - let (if_var, if_op, if_value) = extract_if_condition(if_stmt)?; - eprintln!("[joinir/pattern3/if-sum] If condition: {} {:?} {}", if_var, if_op, if_value); + eprintln!( + "[joinir/pattern3/if-sum] Extracted {} condition variables: {:?}", + cond_vars.len(), + cond_vars + ); + + // Phase 220-B Step 2: Build ConditionEnv using ConditionEnvBuilder pattern + let mut cond_env = ConditionEnv::new(); + let mut cond_bindings = Vec::new(); + + // Add loop variable to environment + let loop_var_join_id = join_value_space.alloc_param(); + cond_env.insert(loop_var_name.to_string(), loop_var_join_id); + + eprintln!( + "[joinir/pattern3/if-sum] Loop variable '{}': host={:?}, join={:?}", + loop_var_name, loop_var_id, loop_var_join_id + ); + + // Add condition-only variables + for var_name in &cond_vars { + let host_id = variable_map + .get(var_name) + .copied() + .ok_or_else(|| { + format!( + "[if-sum] Condition variable '{}' not found in variable_map. \ + Available: {:?}", + var_name, + variable_map.keys().collect::>() + ) + })?; + + let join_id = join_value_space.alloc_param(); + cond_env.insert(var_name.clone(), join_id); + cond_bindings.push(ConditionBinding { + name: var_name.clone(), + host_value: host_id, + join_value: join_id, + }); + + eprintln!( + "[joinir/pattern3/if-sum] Condition variable '{}': host={:?}, join={:?}", + var_name, host_id, join_id + ); + } + + eprintln!( + "[joinir/pattern3/if-sum] ConditionEnv built: {} variables, {} bindings", + cond_env.len(), + cond_bindings.len() + ); + + // Step 3: Extract loop condition info (e.g., i < len → var="i", op=Lt, limit=len/ValueId) + let (loop_var, loop_op, loop_limit) = extract_loop_condition(loop_condition, &cond_env)?; + eprintln!("[joinir/pattern3/if-sum] Loop condition: {} {:?} {:?}", loop_var, loop_op, loop_limit); + + // Step 4: Extract if condition info (e.g., i > 0 → var="i", op=Gt, value=0) + let (if_var, if_op, if_value) = extract_if_condition(if_stmt, &cond_env)?; + let if_value_desc = match &if_value { + ValueOrLiteral::Literal(n) => format!("literal {}", n), + ValueOrLiteral::Variable(name, id) => format!("variable '{}' ({:?})", name, id), + }; + eprintln!("[joinir/pattern3/if-sum] If condition: {} {:?} {}", if_var, if_op, if_value_desc); // Step 3: Extract then-branch update (e.g., sum = sum + 1 → var="sum", addend=1) let (update_var, update_addend) = extract_then_update(if_stmt)?; @@ -137,10 +210,16 @@ pub fn lower_if_sum_pattern( value: ConstValue::Integer(0), })); - // result = loop_step(i_init, sum_init, count_init) + // Phase 220-B: Build call args including condition-only variables + // result = loop_step(i_init, sum_init, count_init, ...condition_vars) + let mut loop_call_args = vec![i_init_val, sum_init_val, count_init_val]; + for binding in &cond_bindings { + loop_call_args.push(binding.join_value); // Include condition vars + } + main_func.body.push(JoinInst::Call { func: loop_step_id, - args: vec![i_init_val, sum_init_val, count_init_val], + args: loop_call_args, k_next: None, dst: Some(loop_result), }); @@ -151,26 +230,53 @@ pub fn lower_if_sum_pattern( join_module.add_function(main_func); - // === loop_step(i, sum, count) function === + // === loop_step function === + // Phase 220-B: Include condition-only variables as parameters + let mut loop_step_params = vec![loop_var_join_id, sum_param, count_param]; + + // Add condition-only variables as parameters + for binding in &cond_bindings { + loop_step_params.push(binding.join_value); + } + + eprintln!( + "[if-sum/joinir] loop_step params: {} total ({} carriers + {} condition vars)", + loop_step_params.len(), + 3, // i, sum, count + cond_bindings.len() + ); + let mut loop_step_func = JoinFunction::new( loop_step_id, "loop_step".to_string(), - vec![i_param, sum_param, count_param], + loop_step_params, ); // --- Exit Condition Check --- - // Load loop limit from AST - loop_step_func.body.push(JoinInst::Compute(MirLikeInst::Const { - dst: loop_limit_val, - value: ConstValue::Integer(loop_limit), - })); + // Phase 220-B: Handle both literal and variable loop limits + let loop_limit_value_id = match loop_limit { + ValueOrLiteral::Literal(n) => { + // Literal: Generate Const instruction + loop_step_func.body.push(JoinInst::Compute(MirLikeInst::Const { + dst: loop_limit_val, + value: ConstValue::Integer(n), + })); + loop_limit_val + } + ValueOrLiteral::Variable(ref var_name, value_id) => { + // Variable: Use ValueId from ConditionEnv (already a parameter) + eprintln!("[if-sum/joinir] Loop limit is variable '{}' = {:?}", var_name, value_id); + value_id + } + }; // Compare: i < limit (or other op from AST) + // Phase 220-B: Use loop_var_join_id (from ConditionEnv) instead of i_param loop_step_func.body.push(JoinInst::Compute(MirLikeInst::Compare { dst: cmp_loop, op: loop_op, - lhs: i_param, - rhs: loop_limit_val, + lhs: loop_var_join_id, // Phase 220-B: Use ConditionEnv ValueId + rhs: loop_limit_value_id, })); // exit_cond = !cmp_loop @@ -188,18 +294,30 @@ pub fn lower_if_sum_pattern( }); // --- If Condition (AST-based) --- - // Load if constant - loop_step_func.body.push(JoinInst::Compute(MirLikeInst::Const { - dst: if_const, - value: ConstValue::Integer(if_value), - })); + // Phase 220-B: Handle both literal and variable if conditions + let if_value_id = match if_value { + ValueOrLiteral::Literal(n) => { + // Literal: Generate Const instruction + loop_step_func.body.push(JoinInst::Compute(MirLikeInst::Const { + dst: if_const, + value: ConstValue::Integer(n), + })); + if_const + } + ValueOrLiteral::Variable(ref var_name, value_id) => { + // Variable: Use ValueId from ConditionEnv (already a parameter) + eprintln!("[if-sum/joinir] If condition value is variable '{}' = {:?}", var_name, value_id); + value_id + } + }; // Compare: if_var if_value + // Phase 220-B: Use loop_var_join_id (from ConditionEnv) loop_step_func.body.push(JoinInst::Compute(MirLikeInst::Compare { dst: if_cmp, op: if_op, - lhs: i_param, // Assuming if_var == loop_var (common case) - rhs: if_const, + lhs: loop_var_join_id, // Phase 220-B: Use ConditionEnv ValueId + rhs: if_value_id, })); // --- Then Branch --- @@ -269,17 +387,24 @@ pub fn lower_if_sum_pattern( dst: step_const2, value: ConstValue::Integer(counter_step), })); + // Phase 220-B: Use loop_var_join_id for counter increment loop_step_func.body.push(JoinInst::Compute(MirLikeInst::BinOp { dst: i_next, op: BinOpKind::Add, - lhs: i_param, + lhs: loop_var_join_id, // Phase 220-B: Use ConditionEnv ValueId rhs: step_const2, })); // --- Tail Recursion --- + // Phase 220-B: Include condition-only variables in recursive call + let mut recursive_args = vec![i_next, sum_new, count_new]; + for binding in &cond_bindings { + recursive_args.push(binding.join_value); // Pass condition vars through + } + loop_step_func.body.push(JoinInst::Call { func: loop_step_id, - args: vec![i_next, sum_new, count_new], + args: recursive_args, k_next: None, dst: None, }); @@ -312,21 +437,29 @@ pub fn lower_if_sum_pattern( let fragment_meta = JoinFragmentMeta::with_expr_result(sum_final, exit_meta); eprintln!("[joinir/pattern3/if-sum] Generated AST-based JoinIR"); - eprintln!("[joinir/pattern3/if-sum] Loop: {} {:?} {}", loop_var, loop_op, loop_limit); - eprintln!("[joinir/pattern3/if-sum] If: {} {:?} {}", if_var, if_op, if_value); + let loop_limit_desc = match &loop_limit { + ValueOrLiteral::Literal(n) => format!("literal {}", n), + ValueOrLiteral::Variable(name, id) => format!("variable '{}' ({:?})", name, id), + }; + eprintln!("[joinir/pattern3/if-sum] Loop: {} {:?} {}", loop_var, loop_op, loop_limit_desc); + eprintln!("[joinir/pattern3/if-sum] If: {} {:?} {}", if_var, if_op, if_value_desc); eprintln!("[joinir/pattern3/if-sum] Phase 215-2: expr_result={:?}", sum_final); + eprintln!("[joinir/pattern3/if-sum] Phase 220-B: Returning {} condition bindings", cond_bindings.len()); - Ok((join_module, fragment_meta)) + Ok((join_module, fragment_meta, cond_bindings)) } -/// Extract loop condition: variable, operator, and limit +/// Phase 220-B: Extract loop condition with variable support /// -/// Supports: `var < lit`, `var <= lit`, `var > lit`, `var >= lit` -fn extract_loop_condition(cond: &ASTNode) -> Result<(String, CompareOp, i64), String> { +/// Supports: `var < lit/var`, `var <= lit/var`, `var > lit/var`, `var >= lit/var` +fn extract_loop_condition( + cond: &ASTNode, + cond_env: &ConditionEnv, +) -> Result<(String, CompareOp, ValueOrLiteral), String> { match cond { ASTNode::BinaryOp { operator, left, right, .. } => { let var_name = extract_variable_name(left)?; - let limit = extract_integer_literal(right)?; + let limit = extract_value_or_variable(right, cond_env)?; let op = match operator { crate::ast::BinaryOperator::Less => CompareOp::Lt, crate::ast::BinaryOperator::LessEqual => CompareOp::Le, @@ -340,11 +473,14 @@ fn extract_loop_condition(cond: &ASTNode) -> Result<(String, CompareOp, i64), St } } -/// Extract if condition: variable, operator, and value -fn extract_if_condition(if_stmt: &ASTNode) -> Result<(String, CompareOp, i64), String> { +/// Phase 220-B: Extract if condition with variable support +fn extract_if_condition( + if_stmt: &ASTNode, + cond_env: &ConditionEnv, +) -> Result<(String, CompareOp, ValueOrLiteral), String> { match if_stmt { ASTNode::If { condition, .. } => { - extract_loop_condition(condition) // Same format + extract_loop_condition(condition, cond_env) // Same format } _ => Err("[if-sum] Expected If statement".to_string()), } @@ -406,15 +542,54 @@ fn extract_variable_name(node: &ASTNode) -> Result { } } -/// Extract integer literal from AST node +/// Phase 220-B: Value or literal enum for condition expressions +#[derive(Debug)] +enum ValueOrLiteral { + Literal(i64), + Variable(String, ValueId), +} + +/// Helper: Extract integer literal (for non-condition contexts like arithmetic) fn extract_integer_literal(node: &ASTNode) -> Result { match node { ASTNode::Literal { value: crate::ast::LiteralValue::Integer(n), .. } => Ok(*n), ASTNode::Variable { name, .. } => { - // Handle variable reference (e.g., `len`) - // For Phase 213, we only support literals. Variables need Phase 214+ - Err(format!("[if-sum] Variable '{}' in condition not supported yet (Phase 214+)", name)) + Err(format!("[if-sum] Variable '{}' in arithmetic expression not supported yet", name)) } _ => Err(format!("[if-sum] Expected integer literal, got {:?}", node)), } } + +/// Phase 220-B: Extract value or variable from AST node (with ConditionEnv support) +fn extract_value_or_variable( + node: &ASTNode, + cond_env: &ConditionEnv, +) -> Result { + match node { + // Literal: Return as immediate value + ASTNode::Literal { value: crate::ast::LiteralValue::Integer(n), .. } => { + Ok(ValueOrLiteral::Literal(*n)) + } + + // Variable: Lookup in ConditionEnv + ASTNode::Variable { name, .. } => { + if let Some(value_id) = cond_env.get(name) { + eprintln!("[if-sum/extract] Variable '{}' resolved to {:?}", name, value_id); + Ok(ValueOrLiteral::Variable(name.clone(), value_id)) + } else { + // Fail-Fast: Variable not in ConditionEnv + Err(format!( + "[if-sum] Variable '{}' not found in ConditionEnv (available: {:?})", + name, cond_env.names() + )) + } + } + + // Method call: Not supported yet (defer to Phase 221+) + ASTNode::MethodCall { .. } => { + Err("[if-sum] Method calls in conditions not supported yet (Phase 221+)".to_string()) + } + + _ => Err(format!("[if-sum] Expected integer literal or variable, got {:?}", node)), + } +}