diff --git a/docs/development/current/main/phase189-select-instruction-inquiry.md b/docs/development/current/main/phase189-select-instruction-inquiry.md new file mode 100644 index 00000000..82d0f016 --- /dev/null +++ b/docs/development/current/main/phase189-select-instruction-inquiry.md @@ -0,0 +1,352 @@ +# Phase 189: Select Instruction MIR Bridge - ChatGPT Architectural Inquiry + +**Date**: 2025-12-05 +**Status**: Blocking Issue Identified +**Assigned to**: ChatGPT (Architecture & Design Guidance) +**Related Phase**: Phase 188-Impl-3 (Pattern 3 implementation complete except for MIR bridge) + +--- + +## Executive Summary + +Phase 188-Impl-3 (Loop with If-Else PHI) has successfully implemented: +- ✅ Pattern 3 JoinIR lowering infrastructure (loop_with_if_phi_minimal.rs) +- ✅ Pattern 3 routing in MIR builder (control_flow.rs) +- ✅ Select instruction definition in JoinIR +- ✅ JoinIR runtime execution for Select +- ✅ JSON serialization for Select + +**Remaining Blocker**: Pattern 3 tests cannot execute because **Select instruction conversion from JoinIR to MIR is not implemented**. + +The Select instruction (JoinIR's ternary operator: `cond ? then_val : else_val`) cannot be directly represented in MIR's current instruction set. We need a clean architectural approach to convert it to MIR-compatible control flow. + +--- + +## Current Situation + +### What Works (Pattern 1 & 2) +- Pattern 1 (Simple While Loop): ✅ Fully working (loop_min_while.hako) +- Pattern 2 (Loop with Conditional Break): ✅ Fully working (joinir_min_loop.hako) +- Both use only instructions already implemented in MIR bridge + +### What Doesn't Work (Pattern 3) +- Pattern 3 (Loop with If-Else PHI): 🔄 Infrastructure complete, execution blocked +- Test case: apps/tests/loop_if_phi.hako +- Expected: Prints "sum=9" (sum of odd numbers 1-5) +- Actual: Fails at MIR bridge conversion with Select instruction + +### Root Cause: Missing Select → MIR Conversion + +```rust +// JoinIR lowering generates this (works fine): +let sum_new = alloc_value(); +loop_step_func.body.push(JoinInst::Compute(MirLikeInst::Select { + dst: sum_new, + cond: if_cond, + then_val: sum_then, + else_val: sum_else, +})); + +// MIR bridge conversion fails here: +// src/mir/join_ir_vm_bridge/convert.rs line ~XXX +MirLikeInst::Select { .. } => { + return Err("Select instruction not yet implemented".into()); + // ❌ This is where we get stuck +} +``` + +### Why Select Can't Be Direct in MIR + +MIR instruction set (14 instructions) doesn't have a direct ternary/select instruction: +- **Arithmetic**: Const, UnaryOp, BinOp, Compare, TypeOp +- **Memory**: Load, Store +- **Control**: Branch (goto two targets), Jump (goto one target), Return +- **Other**: Phi, NewBox, BoxCall + +**Problem**: Select needs to produce a value *and* involve control flow, which MIR separates: +- Control flow → Branch/Jump instructions that create multiple basic blocks +- Value production → Phi nodes at merge points + +--- + +## Technical Challenge: Select → MIR Conversion + +### Pattern of the Conversion + +A JoinIR Select must be converted to: +1. **Branch block** with the condition → splits into then_block and else_block +2. **Then block** → computes `then_val` +3. **Else block** → computes `else_val` +4. **Merge block** → Phi instruction that merges the two values + +### Example: JoinIR vs MIR Representation + +**JoinIR (what we have)**: +```rust +let if_cond = ... // compute condition +let sum_then = sum + i // then branch +let sum_else = sum + 0 // else branch +let sum_new = Select { // ternary select + cond: if_cond, + then_val: sum_then, + else_val: sum_else, +} +// sum_new is ready to use +``` + +**MIR (what we need to generate)**: +``` +block_before: + if_cond = Compare(...) + Branch if_cond → block_then, block_else + +block_then: + sum_then = BinOp(Add, sum, i) + Jump → block_merge [sum_then] + +block_else: + sum_else = BinOp(Add, sum, 0) + Jump → block_merge [sum_else] + +block_merge: + sum_new = Phi([sum_then, sum_else]) // merges two values + // sum_new ready to use +``` + +--- + +## Questions for ChatGPT + +### 1. **Clean Conversion Strategy** +How should we cleanly structure the Select → MIR conversion in the codebase? + +**Considerations**: +- Should this be a separate transformation function or inline in the converter? +- What's the best way to handle the block generation and control flow linking? +- Should we use existing utilities (e.g., block management functions) or create new ones? + +**Reference**: Look at how existing Select-like constructs are handled: +- If/Then/Else lowering in `src/mir/builder/if_builder.rs` (lines ~200-350) +- Phi generation in `src/mir/join_ir/lowering/if_select.rs` +- How Branch/Phi are currently being connected + +### 2. **Block Creation and Management** +The conversion needs to create new MIR blocks. How should this interact with existing block management? + +**Questions**: +- Should new blocks be pre-allocated (like LoopForm does)? +- Should the conversion happen immediately or deferred? +- How do we ensure proper linking between blocks (edges, dominance)? +- Do we need metadata tracking for Select block origins? + +### 3. **Value ID Continuity** +JoinIR uses local ValueIds (0, 1, 2, ...), which get remapped to host ValueIds. How do we handle the intermediate values? + +**Context**: +- JoinModule is converted to MirModule via `convert_join_module_to_mir_with_meta()` +- ValueIds are already remapped at this stage +- Then blocks are merged via `merge_joinir_mir_blocks()` + +**Question**: Should the Select expansion happen: +- (A) In `convert_join_module_to_mir_with_meta()` (early in the JoinModule→MirModule stage)? +- (B) In the MIR bridge converter (current location)? +- (C) In `merge_joinir_mir_blocks()` (late, after remapping)? + +### 4. **Code Organization** +Where should the Select conversion logic live? + +**Options**: +- Option A: New file `src/mir/join_ir_vm_bridge/select_expansion.rs` + - Pros: Single responsibility, easy to test, clear naming + - Cons: Small file, might split related code +- Option B: Expand `src/mir/join_ir_vm_bridge/convert.rs` + - Pros: Centralized conversion logic + - Cons: File gets larger, mixed concerns +- Option C: Create in JoinIR lowering layer (`src/mir/join_ir/lowering/select_expansion.rs`) + - Pros: Closer to where Select is created + - Cons: Mixing JoinIR and MIR concerns + +**What's the architectural pattern used elsewhere in the codebase?** + +### 5. **Performance and Optimization Implications** +Select expansion creates extra blocks and Phi nodes. Any concerns? + +**Questions**: +- Will the VM interpreter handle this correctly? +- Does the LLVM backend optimize this back to conditional moves? +- Should we add a Select-fast-path for simple cases (where Select result isn't used in loops)? +- How do we measure the quality of generated MIR? + +### 6. **Testing Strategy** +How should we validate the Select expansion? + +**Suggested Approach**: +- Existing test: apps/tests/loop_if_phi.hako (integration test) +- New unit test: Test Select expansion in isolation +- MIR output inspection: Compare before/after +- Round-trip test: JoinIR → MIR → VM execution + +**Your input**: What's the minimal test to validate correctness? + +### 7. **Future Extensibility** +Pattern 3 uses Select for single variable mutation. What about more complex patterns? + +**Consider**: +- Pattern 4+: Multiple variables mutating in if/else? +- IfMerge instruction (which we have but don't use)? +- Should IfMerge replace Select as the canonical form? +- How does this relate to Phase 33's IfMerge lowering work? + +--- + +## Existing References in Codebase + +### Similar Transformations + +1. **If-Select lowering** (`src/mir/join_ir/lowering/if_select.rs`) + - Converts if/else with PHI to JoinIR Select instruction + - ~180 lines, well-structured + - **Insight**: This is the *opposite* direction (MIR→JoinIR Select) + +2. **If builder** (`src/mir/builder/if_builder.rs`) + - Creates MIR blocks for if/then/else + - Lines 200-350 show block creation and Phi handling + - **Insight**: Shows how to properly structure MIR control flow + +3. **Loop builder** (`src/mir/builder/loop_builder.rs`) + - Generates loop control flow with Phi nodes + - Handles block linking and edge management + - **Insight**: Established patterns for block management + +4. **LoopForm** (`src/mir/loop_pattern_detection.rs` + `src/mir/mir_loopform.rs`) + - Entire subsystem for loop pattern detection + - Pre-allocates and links blocks + - **Insight**: Large-scale block transformation pattern + +### Related Files +- `src/mir/join_ir_vm_bridge/convert.rs` (current location of Select error) +- `src/mir/join_ir/lowering/inline_boundary.rs` (ValueId mapping) +- `src/mir/join_ir/lowering/loop_patterns.rs` (routing logic) + +--- + +## Proposed Implementation Phases + +### Phase 189-A: Design & Validation (ChatGPT Input) +- Architectural decision on conversion location (early vs late in pipeline) +- Code organization approach +- Test strategy definition + +### Phase 189-B: Core Implementation +- Implement Select → Branch + Then + Else + Phi conversion +- Unit tests for conversion correctness +- Integration test with loop_if_phi.hako + +### Phase 189-C: Optimization & Polish +- Performance validation +- MIR output quality analysis +- Documentation and comments + +--- + +## Success Criteria + +✅ **Minimal**: Pattern 3 test (loop_if_phi.hako) produces `sum=9` +✅ **Better**: All MIR patterns (1-3) work correctly +✅ **Best**: Design is extensible for future patterns (4+) + +--- + +## Timeline Context + +- **Phase 188-Impl-1**: Pattern 1 ✅ Complete (loop_min_while.hako) +- **Phase 188-Impl-2**: Pattern 2 ✅ Complete (joinir_min_loop.hako) +- **Phase 188-Impl-3**: Pattern 3 🔄 Lowering complete, MIR bridge pending +- **Phase 189**: Select expansion (THIS INQUIRY) +- **Phase 190+**: Additional patterns, optimizations, cleanup + +--- + +## Key Files for Reference + +``` +docs/development/current/main/phase188-select-implementation-spec.md ← Spec +docs/private/roadmap2/phases/phase-188-joinir-loop-pattern-expansion/ + ├── design.md ← Full 1648-line design document + └── pattern3-implementation-spec.md ← Pattern 3 detailed spec + +src/mir/join_ir/lowering/ + ├── loop_with_if_phi_minimal.rs ← Pattern 3 lowering (381 lines) + ├── if_select.rs ← Similar conversion (opposite dir) + └── inline_boundary.rs ← ValueId mapping + +src/mir/join_ir_vm_bridge/convert.rs ← Current error location (line ~XXX) + +src/mir/builder/ + ├── if_builder.rs ← Example: if/else block creation + └── loop_builder.rs ← Example: loop block management + +apps/tests/loop_if_phi.hako ← Pattern 3 test (blocked) +``` + +--- + +## Appendix: Test Case Details + +### Pattern 3 Test: loop_if_phi.hako +```nyash +static box Main { + main(args) { + local console = new ConsoleBox() + local i = 1 + local sum = 0 + loop(i <= 5) { + if (i % 2 == 1) { sum = sum + i } else { sum = sum + 0 } + i = i + 1 + } + console.println("sum=" + sum) + return 0 + } +} +``` + +**Execution trace**: +- i=1: 1%2==1 → sum=0+1=1 +- i=2: 2%2==0 → sum=1+0=1 +- i=3: 3%2==1 → sum=1+3=4 +- i=4: 4%2==0 → sum=4+0=4 +- i=5: 5%2==1 → sum=4+5=9 +- i=6: 6<=5 is false → exit, print "sum=9" + +**Expected output**: `sum=9\n` + +--- + +## ChatGPT Requested Deliverables + +1. **Architectural Recommendation** (Section 1-4) + - Clean conversion strategy + - Code organization approach + - Block management pattern + - ValueId handling strategy + +2. **Design Document** (reference material provided) + - Template for Phase 189-A design writeup + - Implementation checklist + +3. **Code Pattern Examples** (if helpful) + - Reference snippets from similar transformations + - Pseudocode for Select expansion algorithm + +4. **Phase 189 Kickoff Plan** + - Implementation order + - Testing approach + - Success metrics + +--- + +**Status**: Awaiting ChatGPT input on architectural approach before proceeding to Phase 189-B implementation. + +🤖 **Created by**: Claude Code +📅 **Date**: 2025-12-05 +🎯 **Target**: Phase 189 - Select Instruction MIR Bridge Implementation