Proper HOST↔JoinIR ValueId separation for condition variables: - Add ConditionEnv struct (name → JoinIR-local ValueId mapping) - Add ConditionBinding struct (HOST/JoinIR ValueId pairs) - Modify condition_to_joinir to use ConditionEnv instead of builder.variable_map - Update Pattern2 lowerer to build ConditionEnv and ConditionBindings - Extend JoinInlineBoundary with condition_bindings field - Update BoundaryInjector to inject Copy instructions for condition variables This fixes the undefined ValueId errors where HOST ValueIds were being used directly in JoinIR instructions. Programs now execute (RC: 0), though loop variable exit values still need Phase 172 work. Key invariants established: 1. JoinIR uses ONLY JoinIR-local ValueIds 2. HOST↔JoinIR bridging is ONLY through JoinInlineBoundary 3. condition_to_joinir NEVER accesses builder.variable_map 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
8.8 KiB
Phase 171: JoinIR → MIR ValueId Boundary Mapping Fix - Completion Report
Date: 2025-12-07 Status: ✅ Infrastructure Complete (Testing in Progress)
Executive Summary
Successfully implemented the infrastructure to map condition-only input variables across the JoinIR → MIR boundary. This fixes the root cause of "undefined ValueId" errors for variables like start, end, len that appear only in loop conditions.
Problem Solved: Variables used only in loop conditions (not loop parameters) were using HOST ValueIds directly in JoinIR, causing undefined value errors when merged into MIR.
Solution Implemented: Extended JoinInlineBoundary with condition_inputs field and updated the entire merge pipeline to collect, remap, and inject Copy instructions for these variables.
Implementation Phases Completed
✅ Phase 171-1: Boundary Coverage Analysis
Deliverable: phase171-1-boundary-analysis.md (documented current state)
Key Findings:
- Loop variables (
i): ✅ Properly mapped viajoin_inputs - Carriers (
sum,count): ✅ Properly mapped viaexit_bindings - Condition inputs (
start,end): ❌ NOT MAPPED → ROOT CAUSE
✅ Phase 171-2: Design Decision
Deliverable: phase171-2-condition-inputs-design.md
Decision: Option A - Extend JoinInlineBoundary
Added field:
pub condition_inputs: Vec<(String, ValueId)>
Rationale:
- Minimal invasiveness
- Clear semantics
- Reuses existing Copy injection mechanism
- Future-proof design
✅ Phase 171-3: Infrastructure Implementation
Deliverable: phase171-3-implementation-report.md
Files Modified:
-
inline_boundary.rs(+93 lines)- Added
condition_inputsfield - Added 2 new constructors
- Updated 4 existing constructors
- Added
-
condition_to_joinir.rs(+180 lines)- Implemented
extract_condition_variables() - Added recursive AST traversal
- Added 3 comprehensive tests
- Implemented
-
exit_binding.rs(+2 lines)- Fixed test boundary initialization
Build Status: ✅ Successful (0 errors, 57 warnings)
✅ Phase 171-4: Merge Logic Integration
Files Modified:
-
pattern2_with_break.rs(+19 lines)- Extract condition variables from AST
- Look up HOST ValueIds
- Pass to boundary constructor
-
merge/mod.rs(+13 lines)- Add condition_inputs to used_values for remapping
- Debug logging for condition inputs
-
merge/instruction_rewriter.rs(+17 lines)- Add condition_inputs to value_map_for_injector
- Enable remapping of HOST ValueIds
-
joinir_inline_boundary_injector.rs(+35 lines)- Inject Copy instructions for condition inputs
- Handle both join_inputs and condition_inputs
Build Status: ✅ Successful (0 errors, 57 warnings)
Technical Changes Summary
Data Flow: Before vs After
Before (Broken)
HOST: start = ValueId(33)
↓ condition_to_joinir reads directly
JoinIR: uses ValueId(33) in Compare
↓ NO BOUNDARY, NO REMAP
MIR: ValueId(33) undefined → ERROR ❌
After (Fixed)
HOST: start = ValueId(33)
↓ pattern2_with_break extracts "start"
↓ boundary.condition_inputs = [("start", ValueId(33))]
↓ merge adds to used_values
↓ remap_values: ValueId(33) → ValueId(100)
↓ BoundaryInjector: Copy ValueId(100) = Copy ValueId(33)
JoinIR: uses ValueId(33) in Compare
↓ instruction_rewriter remaps to ValueId(100)
MIR: ValueId(100) = Copy ValueId(33) → SUCCESS ✅
Key Implementation Points
1. Condition Variable Extraction (condition_to_joinir.rs:326-361)
pub fn extract_condition_variables(
cond_ast: &ASTNode,
exclude_vars: &[String], // Loop parameters to exclude
) -> Vec<String>
- Recursively traverses condition AST
- Collects unique variable names
- Filters out loop parameters
- Returns sorted list (BTreeSet for determinism)
2. Boundary Registration (pattern2_with_break.rs:73-92)
let condition_var_names = extract_condition_variables(condition, &[loop_var_name.clone()]);
let mut condition_inputs = Vec::new();
for var_name in &condition_var_names {
let host_value_id = self.variable_map.get(var_name)
.copied()
.ok_or_else(|| ...)?;
condition_inputs.push((var_name.clone(), host_value_id));
}
3. Value Collection (merge/mod.rs:80-91)
// Add condition_inputs to used_values for remapping
if let Some(boundary) = boundary {
for (var_name, host_value_id) in &boundary.condition_inputs {
used_values.insert(*host_value_id);
}
}
4. Value Remapping (merge/instruction_rewriter.rs:402-415)
// Add condition_inputs to value_map
for (var_name, host_value_id) in &boundary.condition_inputs {
if let Some(remapped) = remapper.get_value(*host_value_id) {
value_map_for_injector.insert(*host_value_id, remapped);
}
}
5. Copy Injection (joinir_inline_boundary_injector.rs:86-116)
// Inject Copy instructions for condition_inputs
for (var_name, host_value_id) in &boundary.condition_inputs {
if let Some(&remapped_value) = value_map.get(host_value_id) {
let copy_inst = MirInstruction::Copy {
dst: remapped_value,
src: *host_value_id,
};
copy_instructions.push(copy_inst);
}
}
Test Status
✅ Unit Tests Pass
Extraction function tests (condition_to_joinir.rs):
test_extract_condition_variables_simple✅test_extract_condition_variables_with_exclude✅test_extract_condition_variables_complex✅
🔄 Integration Tests (In Progress)
Test File: local_tests/test_trim_main_pattern.hako
Current Status: Still shows undefined ValueId errors:
[ssa-undef-debug] fn=TrimTest.trim/1 bb=BasicBlockId(8) inst_idx=0 used=ValueId(33)
[ssa-undef-debug] fn=TrimTest.trim/1 bb=BasicBlockId(17) inst_idx=0 used=ValueId(49)
Next Debugging Steps:
- Add debug output to Pattern 2 lowerer to confirm condition variable extraction
- Verify boundary condition_inputs are populated
- Check value remapping in merge logic
- Verify Copy instruction injection
Remaining Work
Phase 171-5: Verification & Documentation
Tasks:
- ✅ Unit tests complete
- 🔄 Integration tests (debugging in progress)
- ⏳ Update CURRENT_TASK.md
- ⏳ Final documentation
Blocker: Need to debug why ValueId(33) is still undefined despite infrastructure being in place.
Hypothesis: The issue might be:
- Condition variable extraction not running (check debug output)
- Boundary not being passed correctly
- Value remapping not applying to all instruction types
- Copy instructions not being injected
Files Modified (Summary)
| File | Lines Changed | Purpose |
|---|---|---|
inline_boundary.rs |
+93 | Add condition_inputs field + constructors |
condition_to_joinir.rs |
+180 | Extract condition variables from AST |
exit_binding.rs |
+2 | Fix test |
pattern2_with_break.rs |
+19 | Register condition inputs |
merge/mod.rs |
+13 | Add to used_values |
merge/instruction_rewriter.rs |
+17 | Add to value_map |
joinir_inline_boundary_injector.rs |
+35 | Inject Copy instructions |
| Total | +359 lines | Complete infrastructure |
Design Principles Applied
-
Box-First Philosophy ✅
- Clean separation: JoinIR frontier, boundary metadata, merge logic
- Each component has single responsibility
-
Fail-Fast ✅
- Explicit error when condition variable not in variable_map
- No silent fallbacks
-
Determinism ✅
- BTreeSet for sorted variable names
- Consistent iteration order
-
80/20 Rule ✅
- Infrastructure first (80%)
- Debugging and edge cases (20% - in progress)
Known Limitations
-
Pattern 1/3/4 Not Yet Updated
- Only Pattern 2 (loop_with_break) currently implements condition input extraction
- Other patterns will need similar updates
-
No Condition-Only Outputs
- Current design handles inputs only
- Outputs would need similar treatment (future extension point)
-
Manual Debugging Required
- Integration tests not yet passing
- Need to trace execution to find where mapping breaks down
Next Steps
Immediate:
- Add debug output to confirm condition variable extraction runs
- Verify boundary.condition_inputs is populated
- Check if Copy instructions are actually injected
- Trace ValueId remapping through merge pipeline
Follow-up:
- Update Pattern 1/3/4 lowerers with same infrastructure
- Add integration tests for each pattern
- Document edge cases and limitations
- Update CURRENT_TASK.md with completion status
References
- Phase 171-1 Analysis:
phase171-1-boundary-analysis.md - Phase 171-2 Design:
phase171-2-condition-inputs-design.md - Phase 171-3 Implementation:
phase171-3-implementation-report.md - Phase 170 Background:
phase170-completion-report.md