e2508f9f08
Implements Phase 215 Task 215-2: Wire expr_result exit line through ExitMeta → JoinInlineBoundary → ExitLine → MIR return for Pattern 3. ## Changes ### Fix 1: JoinIR Lowerer (loop_with_if_phi_if_sum.rs:312) - Changed `carrier_only()` → `with_expr_result(sum_final, exit_meta)` - Marks sum final value as expr_result for propagation ### Fix 2: Boundary Builder (pattern3_with_if_phi.rs:176-191) - Added `.with_expr_result(Some(expr_id))` to JoinInlineBoundaryBuilder - Passes expr_result to boundary for ExitLineReconnector ### Fix 3: Final Return (pattern3_with_if_phi.rs:195-219) - Changed from always returning Void to conditional return: - Expr-position loops: Return merge_result ValueId - Statement-position loops: Return Void - Matches Pattern 2 behavior ## Test Results Primary target: - phase212_if_sum_min.hako: RC=2 achieved ✅ Regression tests: - loop_if_phi.hako: RC=2 (existing behavior maintained) ✅ - Pattern 1/2/3 tests: All PASS ✅ ## Architecture Pattern 3 now follows the same ExprResult Exit Contract as Pattern 2: 1. JoinIR lowerer creates expr_result with `with_expr_result()` 2. Boundary builder passes expr_result with `.with_expr_result()` 3. Conversion pipeline returns merge result containing exit PHI ValueId 4. Pattern dispatcher conditionally returns expr_result or Void This enables: - Expr-position loops: Loop result propagates to caller - Statement-position loops: Loop updates variable_map only (returns Void) - Unified contract: Patterns 2 and 3 follow same expr_result flow 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
312 lines
16 KiB
Markdown
312 lines
16 KiB
Markdown
# Phase 215: ExprResult Exit Contract
|
|
|
|
## Overview
|
|
|
|
Phase 215 establishes a unified ExprResult Exit Contract to properly propagate loop computation results from JoinIR lowerers through to the final MIR return statement.
|
|
|
|
**Problem**: Loop computations (e.g., `sum=9` in if-sum pattern) execute correctly but return RC=0 instead of the expected value because `expr_result` gets discarded at the final return statement.
|
|
|
|
**Goal**: Make `expr_result` with `ParamRole::ExprResult` propagate consistently from JoinIR lowerers → ExitMeta → Boundary → ExitLine → MIR return statement.
|
|
|
|
## Current State Analysis
|
|
|
|
### Investigation Results (Phase 215 Task Agent)
|
|
|
|
#### Pattern 1 (Simple While)
|
|
- **File**: `src/mir/join_ir/lowering/simple_while_minimal.rs`
|
|
- **Return Type**: `Option<JoinModule>` (no JoinFragmentMeta)
|
|
- **expr_result Support**: ❌ None
|
|
- **Status**: Intentionally simple, no value propagation needed
|
|
|
|
#### Pattern 2 (Loop with Break)
|
|
- **File**: `src/mir/join_ir/lowering/loop_with_break_minimal.rs`
|
|
- **Return Type**: `(JoinModule, JoinFragmentMeta)`
|
|
- **expr_result Creation**: ✅ Line 502
|
|
```rust
|
|
let fragment_meta = JoinFragmentMeta::with_expr_result(i_exit, exit_meta);
|
|
```
|
|
- **Boundary Passing**: ✅ Line 447
|
|
```rust
|
|
.with_expr_result(fragment_meta.expr_result)
|
|
```
|
|
- **Final Return**: ❌ Lines 461-468 - **DISCARDS expr_result**
|
|
```rust
|
|
// Phase 188-Impl-2: Return Void (loops don't produce values)
|
|
// The subsequent 'return i' statement will emit its own Load + Return
|
|
let void_val = crate::mir::builder::emission::constant::emit_void(self);
|
|
trace::trace().debug("pattern2", &format!("Loop complete, returning Void {:?}", void_val));
|
|
Ok(Some(void_val))
|
|
```
|
|
- **Root Cause**: Design assumes loops update `variable_map` and subsequent statements read from it
|
|
|
|
#### Pattern 3 (If-PHI)
|
|
- **File**: `src/mir/join_ir/lowering/loop_with_if_phi_minimal.rs`
|
|
- **Return Type**: `(JoinModule, JoinFragmentMeta)`
|
|
- **expr_result Creation**: ❌ Line 427 - **Uses carrier_only()**
|
|
```rust
|
|
let fragment_meta = JoinFragmentMeta::carrier_only(exit_meta);
|
|
```
|
|
- **Boundary Passing**: ❌ No `.with_expr_result()` call (lines 176-180)
|
|
- **Final Return**: ❌ Lines 192-196 - Returns Void
|
|
```rust
|
|
let void_val = crate::mir::builder::emission::constant::emit_void(self);
|
|
trace::trace().debug("pattern3/if-sum", &format!("Loop complete, returning Void {:?}", void_val));
|
|
Ok(Some(void_val))
|
|
```
|
|
- **Status**: **Primary target for Phase 215 fix**
|
|
|
|
#### Pattern 4 (Loop with Continue)
|
|
- **File**: `src/mir/join_ir/lowering/loop_with_continue_minimal.rs`
|
|
- **Return Type**: `(JoinModule, ExitMeta)` (no JoinFragmentMeta - **inconsistent API**)
|
|
- **expr_result Support**: ❌ None
|
|
- **Status**: API inconsistency, but not primary target
|
|
|
|
## Data Flow Diagram
|
|
|
|
### Current Flow (RC=0 Problem)
|
|
|
|
```
|
|
┌─────────────────────────────────────────────────────────────┐
|
|
│ JoinIR Lowerer (Pattern 3) │
|
|
│ │
|
|
│ lower_loop_with_if_phi_pattern() │
|
|
│ └─> JoinFragmentMeta::carrier_only(exit_meta) │
|
|
│ ├─> exit_meta: ExitMeta { │
|
|
│ │ exit_values: {"sum" → ValueId(1008)} │
|
|
│ │ } │
|
|
│ └─> expr_result: None ← ❌ PROBLEM HERE │
|
|
└─────────────────────────────────────────────────────────────┘
|
|
↓
|
|
┌─────────────────────────────────────────────────────────────┐
|
|
│ Pattern3 Dispatcher (pattern3_with_if_phi.rs) │
|
|
│ │
|
|
│ lower_pattern3_if_sum() / lower_pattern3_legacy() │
|
|
│ └─> JoinInlineBoundaryBuilder::new() │
|
|
│ ├─> .with_inputs(join_inputs, host_inputs) │
|
|
│ ├─> .with_exit_bindings(exit_bindings) │
|
|
│ └─> ❌ NO .with_expr_result() call │
|
|
└─────────────────────────────────────────────────────────────┘
|
|
↓
|
|
┌─────────────────────────────────────────────────────────────┐
|
|
│ JoinIRConversionPipeline │
|
|
│ │
|
|
│ execute(builder, join_module, boundary, ...) │
|
|
│ └─> exit_line_reconnector::reconnect_exit_lines() │
|
|
│ └─> ExitLineReconnector::run() │
|
|
│ └─> Updates variable_map with exit PHIs │
|
|
│ {"sum" → ValueId(r456)} ← Correct! │
|
|
└─────────────────────────────────────────────────────────────┘
|
|
↓
|
|
┌─────────────────────────────────────────────────────────────┐
|
|
│ Pattern3 Final Return (pattern3_with_if_phi.rs) │
|
|
│ │
|
|
│ let void_val = emit_void(self); │
|
|
│ Ok(Some(void_val)) ← ❌ DISCARDS expr_result! │
|
|
│ │
|
|
│ Result: RC=0 (Void type) │
|
|
└─────────────────────────────────────────────────────────────┘
|
|
```
|
|
|
|
### Target Flow (Phase 215 Goal)
|
|
|
|
```
|
|
┌─────────────────────────────────────────────────────────────┐
|
|
│ JoinIR Lowerer (Pattern 3) │
|
|
│ │
|
|
│ lower_loop_with_if_phi_pattern() │
|
|
│ └─> ✅ JoinFragmentMeta::with_expr_result( │
|
|
│ sum_final_value, ← "sum" carrier final value │
|
|
│ exit_meta │
|
|
│ ) │
|
|
│ ├─> exit_meta: ExitMeta { │
|
|
│ │ exit_values: {"sum" → ValueId(1008)} │
|
|
│ │ } │
|
|
│ └─> expr_result: Some(ValueId(1008)) ← ✅ FIX 1 │
|
|
└─────────────────────────────────────────────────────────────┘
|
|
↓
|
|
┌─────────────────────────────────────────────────────────────┐
|
|
│ Pattern3 Dispatcher (pattern3_with_if_phi.rs) │
|
|
│ │
|
|
│ lower_pattern3_if_sum() / lower_pattern3_legacy() │
|
|
│ └─> JoinInlineBoundaryBuilder::new() │
|
|
│ ├─> .with_inputs(join_inputs, host_inputs) │
|
|
│ ├─> .with_exit_bindings(exit_bindings) │
|
|
│ └─> ✅ .with_expr_result(fragment_meta.expr_result) │
|
|
│ ← FIX 2: Pass expr_result to boundary │
|
|
└─────────────────────────────────────────────────────────────┘
|
|
↓
|
|
┌─────────────────────────────────────────────────────────────┐
|
|
│ JoinIRConversionPipeline │
|
|
│ │
|
|
│ execute(builder, join_module, boundary, ...) │
|
|
│ ├─> Merge blocks (allocates PHI nodes) │
|
|
│ │ └─> exit_phi_result_id = Some(ValueId(r999)) │
|
|
│ │ ↑ │
|
|
│ │ └─ This is the merged expr_result PHI! │
|
|
│ │ │
|
|
│ └─> exit_line_reconnector::reconnect_exit_lines() │
|
|
│ └─> ExitLineReconnector::run() │
|
|
│ ├─> Updates variable_map with exit PHIs │
|
|
│ │ {"sum" → ValueId(r456)} │
|
|
│ │ │
|
|
│ └─> ✅ Returns exit_phi_result_id │
|
|
│ Some(ValueId(r999)) ← FIX 3 │
|
|
└─────────────────────────────────────────────────────────────┘
|
|
↓
|
|
┌─────────────────────────────────────────────────────────────┐
|
|
│ Pattern3 Final Return (pattern3_with_if_phi.rs) │
|
|
│ │
|
|
│ let loop_result = JoinIRConversionPipeline::execute(...)?; │
|
|
│ if let Some(result_id) = loop_result { │
|
|
│ ✅ Ok(Some(result_id)) ← FIX 4: Return expr_result! │
|
|
│ } else { │
|
|
│ let void_val = emit_void(self); │
|
|
│ Ok(Some(void_val)) │
|
|
│ } │
|
|
│ │
|
|
│ Result: RC=2 (Integer value) │
|
|
└─────────────────────────────────────────────────────────────┘
|
|
```
|
|
|
|
## Where expr_result Gets Discarded
|
|
|
|
### Primary Discard Points
|
|
|
|
1. **Pattern 3 JoinIR Lowerer** (Line 427)
|
|
- File: `src/mir/join_ir/lowering/loop_with_if_phi_minimal.rs`
|
|
- Code: `JoinFragmentMeta::carrier_only(exit_meta)`
|
|
- Effect: Sets `expr_result = None`, losing the loop result ValueId
|
|
|
|
2. **Pattern 3 Boundary Builder** (Lines 176-180)
|
|
- File: `src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs`
|
|
- Missing: `.with_expr_result()` call
|
|
- Effect: Boundary doesn't know about expr_result
|
|
|
|
3. **Pattern 3 Final Return** (Lines 192-196, 312-316)
|
|
- File: `src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs`
|
|
- Code: `let void_val = emit_void(self); Ok(Some(void_val))`
|
|
- Effect: Discards merge result, returns Void
|
|
|
|
### Secondary Discard Points
|
|
|
|
4. **Pattern 2 Final Return** (Lines 461-468)
|
|
- File: `src/mir/join_ir/lowering/loop_with_break_minimal.rs`
|
|
- Code: Same Void return pattern
|
|
- Note: Pattern 2 correctly creates expr_result but also discards it
|
|
|
|
## Implementation Plan
|
|
|
|
### Task 215-2: Unify expr_result in Lowerers
|
|
|
|
**Pattern 3 Changes**:
|
|
1. Change `JoinFragmentMeta::carrier_only()` to `with_expr_result()` in both:
|
|
- `loop_with_if_phi_minimal.rs` (legacy lowerer)
|
|
- `loop_with_if_phi_if_sum.rs` (if-sum lowerer)
|
|
2. Identify which carrier represents the loop result (e.g., "sum" in if-sum pattern)
|
|
3. Pass that carrier's ValueId as expr_result
|
|
|
|
**Pattern 1/2 Audit**:
|
|
- Pattern 1: No changes needed (intentionally simple)
|
|
- Pattern 2: Already correct, use as reference implementation
|
|
|
|
### Task 215-3: Unify Boundary/ExitLine Handling
|
|
|
|
**Boundary Changes** (`pattern3_with_if_phi.rs`):
|
|
```rust
|
|
let boundary = JoinInlineBoundaryBuilder::new()
|
|
.with_inputs(join_inputs, host_inputs)
|
|
.with_exit_bindings(exit_bindings)
|
|
.with_loop_var_name(Some(ctx.loop_var_name.clone()))
|
|
.with_expr_result(fragment_meta.expr_result) // ← ADD THIS
|
|
.build();
|
|
```
|
|
|
|
**ExitLine Changes** (if needed):
|
|
- Verify `ExitLineReconnector` already handles expr_result correctly
|
|
- Current implementation returns `Option<ValueId>` from merge
|
|
- Should propagate `exit_phi_result_id` when expr_result exists
|
|
|
|
**Final Return Changes** (`pattern3_with_if_phi.rs`):
|
|
```rust
|
|
// Replace Void return with conditional expr_result return
|
|
let merge_result = JoinIRConversionPipeline::execute(...)?;
|
|
|
|
if let Some(result_id) = merge_result {
|
|
// Loop produced a value (expr-position)
|
|
Ok(Some(result_id))
|
|
} else {
|
|
// Loop only updates variable_map (statement-position)
|
|
let void_val = emit_void(self);
|
|
Ok(Some(void_val))
|
|
}
|
|
```
|
|
|
|
### Task 215-4: Testing
|
|
|
|
**Primary Target**:
|
|
- `phase212_if_sum_min.hako` → Expected: RC=2 (sum value)
|
|
|
|
**Regression Tests**:
|
|
- `loop_if_phi.hako` → Expected: sum=9 in variable_map (statement-position)
|
|
- Multi-carrier tests → Expected: All carriers updated correctly
|
|
- Pattern 1/2/4 tests → Expected: No behavioral changes
|
|
|
|
**Verification**:
|
|
- MIR dump: Check final return uses correct ValueId
|
|
- JoinIRVerifier: Ensure no contract violations
|
|
- VM execution: Verify correct RC values
|
|
|
|
### Task 215-5: Documentation
|
|
|
|
**Update Files**:
|
|
1. `phase212-if-sum-impl.md` - Mark Phase 215 as complete
|
|
2. `joinir-architecture-overview.md` - Add ExprResult flow section
|
|
3. `CURRENT_TASK.md` - Record Phase 215 completion
|
|
|
|
## Design Contracts
|
|
|
|
### JoinFragmentMeta Contract
|
|
|
|
```rust
|
|
pub struct JoinFragmentMeta {
|
|
pub expr_result: Option<ValueId>, // Loop result for expr-position
|
|
pub exit_meta: ExitMeta, // Carrier exit values
|
|
}
|
|
```
|
|
|
|
**Usage Rules**:
|
|
- `carrier_only(exit_meta)`: Statement-position loops (updates variable_map only)
|
|
- `with_expr_result(value_id, exit_meta)`: Expr-position loops (returns value)
|
|
|
|
### ParamRole Contract
|
|
|
|
```rust
|
|
pub enum ParamRole {
|
|
LoopParam, // Loop variable (e.g., i)
|
|
Condition, // Loop condition result
|
|
Carrier, // Carrier value (e.g., sum)
|
|
ExprResult, // Final expression result ← New role
|
|
}
|
|
```
|
|
|
|
**Usage Rules**:
|
|
- `Carrier`: Updates `variable_map` via `exit_bindings`
|
|
- `ExprResult`: Returns from loop expression via merge result
|
|
|
|
### ExitLineReconnector Contract
|
|
|
|
**Input**: `JoinInlineBoundary` with optional `expr_result`
|
|
**Output**: `Option<ValueId>` (merge result for expr-position loops)
|
|
|
|
**Behavior**:
|
|
1. Update `variable_map` with all `exit_bindings` (Carrier role)
|
|
2. If `expr_result` exists, return `exit_phi_result_id` (ExprResult role)
|
|
3. Distinguish "carrier to variable_map" vs "expr_result to return"
|
|
|
|
## References
|
|
|
|
- **Phase 213**: AST-based if-sum lowering (dual-mode architecture)
|
|
- **Phase 214**: Dynamic join_inputs generation fix
|
|
- **Phase 188**: Original JoinIR pipeline design
|
|
- **Phase 33-16**: LoopHeaderPhi integration
|