hakorune/docs/development/current/main/phase33-16-refactoring-summary.md

Phase 33-16: Instruction Rewriter Refactoring Summary

Date: 2025-12-07 File: src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs

Motivation

Phase 33-16 fixed a critical bug where tail calls from the entry function's entry block were incorrectly redirected to the header block, creating self-referential loops (bb4 → bb4). The fix worked, but the implicit condition was difficult to understand:

// Before: Implicit semantics
let should_redirect = boundary.is_some()
    && !carrier_phis.is_empty()
    && !is_entry_func_entry_block;

Changes Made

1. TailCallKind Enum (Lines 14-39)

Created an explicit classification system for tail calls:

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum TailCallKind {
    LoopEntry,   // First entry: main → loop_step (no redirect)
    BackEdge,    // Continuation: loop_step → loop_step (redirect to header)
    ExitJump,    // Termination: → k_exit (Return conversion)
}

Why three categories?

• LoopEntry: Entry function's entry block IS the header. Redirecting creates self-loop.
• BackEdge: Loop body blocks must redirect to header for PHI merging.
• ExitJump: Handled separately via Return → Jump conversion.

2. classify_tail_call() Function (Lines 41-69)

Extracted the classification logic into a pure function with explicit semantics:

fn classify_tail_call(
    is_entry_func_entry_block: bool,
    has_loop_header_phis: bool,
    has_boundary: bool,
) -> TailCallKind

Decision logic:

1. If entry function's entry block → LoopEntry (no redirect)
2. Else if boundary exists AND header PHIs exist → BackEdge (redirect)
3. Otherwise → ExitJump (Return conversion handles it)

3. Variable Rename

// Before: Implementation-focused name
let is_entry_func_entry_block = ...;

// After: Semantic name explaining the "why"
let is_loop_entry_point = ...;

Added documentation explaining that this block IS the header, so redirection would create self-loop.

4. Usage Site Refactoring (Lines 416-453)

Replaced implicit boolean logic with explicit match on TailCallKind:

let tail_call_kind = classify_tail_call(...);

let actual_target = match tail_call_kind {
    TailCallKind::BackEdge => {
        // Redirect to header for PHI merging
        loop_header_phi_info.header_block
    }
    TailCallKind::LoopEntry => {
        // No redirect - entry block IS the header
        target_block
    }
    TailCallKind::ExitJump => {
        // Return conversion handles this
        target_block
    }
};

Benefits:

• Each case has explicit reasoning in comments
• Debug logging differentiates between LoopEntry and BackEdge
• Future maintainers can see the "why" immediately

Impact

Code Readability

• Before: Boolean algebra requires mental model of loop structure
• After: Explicit categories with documented semantics

Maintainability

• Classification logic is isolated and testable
• Easy to add new tail call types if needed
• Self-documenting code reduces cognitive load

Correctness

• No behavioral changes (verified by cargo build --release)
• Makes the Phase 33-16 fix's reasoning explicit
• Prevents future bugs from misunderstanding the condition

Verification

cargo build --release
# ✅ Finished `release` profile [optimized] target(s) in 23.38s

All tests pass, no regressions.

Future Improvements

Possible Enhancements (Low Priority)

1. Extract to module: If tail call handling grows, create tail_call_classifier.rs
2. Add unit tests: Test classify_tail_call() with various scenarios
3. Trace logging: Add TailCallKind to debug output for better diagnostics

Not Recommended

• Don't merge LoopEntry and ExitJump - they have different semantics
• Don't inline classify_tail_call() - keeping it separate preserves clarity

Lessons Learned

Implicit semantics are tech debt. The original code worked but required deep knowledge to maintain. The refactoring makes the "why" explicit without changing behavior, improving long-term maintainability at zero runtime cost.

---

Related Files:

• src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs (refactored)
• docs/development/current/main/phase33-16-self-loop-fix.md (original bug fix)