tomoaki/hakorune
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(joinir): Phase 287 P2 - Strengthen BackEdge/latch conditions (WIP)

Browse Source 
**Problem**: Phase 188.3 Pattern6 (nested loop) encounters infinite loop
- inner_step → inner_step (self-recursion) incorrectly classified as BackEdge
- → redirects to outer header (loop_step) instead of inner_step entry
- is_recursive_call causes inner recursion to overwrite outer latch values
- → PHI receives wrong values → i doesn't increment → infinite loop

**Fix 1: BackEdge classification strictness** (tail_call_classifier.rs)
- Add `is_target_loop_entry` parameter to classify_tail_call()
- BackEdge ONLY when target==loop_step (entry_func), not inner_step
- Prevents inner_step → inner_step from redirecting to outer header

**Fix 2: latch_incoming guard** (instruction_rewriter.rs)
- Change condition from `is_recursive_call || is_target_loop_entry`
  to `is_target_loop_entry` only
- Prevents inner_step self-recursion from overwriting outer loop's latch
- set_latch_incoming() now called with correct values (verified by debug)

**Status**: 🚧 WIP - Infinite loop still occurs
- set_latch_incoming('i', BasicBlockId(8), ValueId(21)) ✅ Called correctly
- But final PHI: `phi [%4, bb8]` instead of `phi [%21, bb8]` ❌
- Root cause likely in PHI generation (merge/mod.rs), not latch_incoming
- Next: Investigate why latch_incoming values aren't used in PHI

**Files**:
- src/mir/builder/control_flow/joinir/merge/tail_call_classifier.rs
- src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
tomoaki
2025-12-27 08:32:14 +09:00
parent d0527bcc2a
commit bbfc3c1d94
5 changed files with 105 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs
View File

@ -517,7 +517,9 @@ fn plan_rewrites(
// 1. Loop entry point (header PHIs define carriers)
// 2. Recursive/entry call to loop header with PHIs (latch edge)
// 3. Continuation call (handled separately below)
if is_loop_entry_point {
// Phase 287 P1: Skip ONLY when target is loop header
// (not when source is entry func but target is non-entry like inner_step)
if is_loop_entry_point && is_target_loop_entry {
log!(
verbose,
"[plan_rewrites] Skip param bindings in header block (PHIs define carriers)"
@ -584,9 +586,10 @@ fn plan_rewrites(
}
}
// Record latch incoming for loop header PHI (recursive calls + loop entry calls)
// Phase 188.3: Extended to support continuation→header calls (Pattern6)
if is_recursive_call || is_target_loop_entry {
// Record latch incoming for loop header PHI (only for calls to loop entry func)
// Phase 287 P2: Restrict to is_target_loop_entry only (not is_recursive_call)
// This prevents inner_step recursion from overwriting outer loop's latch values
if is_target_loop_entry {
if let Some(b) = boundary {
if let Some(loop_var_name) = &b.loop_var_name {
if !args.is_empty() {
@ -812,25 +815,34 @@ fn plan_rewrites(
} else if let Some((target_block, args)) = tail_call_target {
// Tail call: Set Jump terminator
// Classify tail call and determine actual target
let is_target_continuation = {
let mut target_func_name: Option<String> = None;
let target_func_name = {
let mut name: Option<String> = None;
for (fname, &entry_block) in &ctx.function_entry_map {
if entry_block == target_block {
target_func_name = Some(fname.clone());
name = Some(fname.clone());
break;
}
}
target_func_name
.as_ref()
.map(|name| continuation_candidates.contains(name))
.unwrap_or(false)
name
};
let is_target_continuation = target_func_name
.as_ref()
.map(|name| continuation_candidates.contains(name))
.unwrap_or(false);
// Phase 287 P2: Compute is_target_loop_entry for classify_tail_call
let is_target_loop_entry = target_func_name
.as_ref()
.map(|name| entry_func_name == Some(name.as_str()))
.unwrap_or(false);
let tail_call_kind = classify_tail_call(
is_loop_entry_point,
!loop_header_phi_info.carrier_phis.is_empty(),
boundary.is_some(),
is_target_continuation,
is_target_loop_entry,
);
let actual_target = match tail_call_kind {

29
src/mir/builder/control_flow/joinir/merge/tail_call_classifier.rs
View File

@ -38,6 +38,7 @@ pub enum TailCallKind {
/// * `has_loop_header_phis` - True if loop header PHI nodes exist
/// * `has_boundary` - True if JoinInlineBoundary exists (indicates loop context)
/// * `is_target_continuation` - True if the tail call target is a continuation function (k_exit)
/// * `is_target_loop_entry` - True if the tail call target is the loop entry function (loop_step)
///
/// # Returns
/// The classification of this tail call
@ -46,6 +47,7 @@ pub fn classify_tail_call(
has_loop_header_phis: bool,
has_boundary: bool,
is_target_continuation: bool,
is_target_loop_entry: bool,
) -> TailCallKind {
// Phase 256 P1.10: Continuation calls (k_exit) are always ExitJump
// They should NOT be redirected to the header block.
@ -60,9 +62,11 @@ pub fn classify_tail_call(
return TailCallKind::LoopEntry;
}
// If we have boundary and header PHIs, this is a back edge
// Must redirect to header for PHI merging
if has_boundary && has_loop_header_phis {
// Phase 287 P2: BackEdge is ONLY for target==loop_step (entry_func)
// This prevents inner_step→inner_step from being classified as BackEdge,
// which would incorrectly redirect it to the outer header (loop_step).
// inner_step→inner_step should jump to inner_step's entry block, not outer header.
if is_target_loop_entry && has_boundary && has_loop_header_phis && !is_entry_func_entry_block {
return TailCallKind::BackEdge;
}
@ -81,6 +85,7 @@ mod tests {
true, // has_loop_header_phis
true, // has_boundary
false, // is_target_continuation
true, // is_target_loop_entry
);
assert_eq!(result, TailCallKind::LoopEntry);
}
@ -92,6 +97,7 @@ mod tests {
true, // has_loop_header_phis
true, // has_boundary
false, // is_target_continuation
true, // is_target_loop_entry ← target is loop entry func
);
assert_eq!(result, TailCallKind::BackEdge);
}
@ -103,6 +109,7 @@ mod tests {
false, // has_loop_header_phis (no header PHIs)
true, // has_boundary
false, // is_target_continuation
false, // is_target_loop_entry
);
assert_eq!(result, TailCallKind::ExitJump);
}
@ -114,6 +121,7 @@ mod tests {
true, // has_loop_header_phis
false, // has_boundary (no boundary → exit)
false, // is_target_continuation
false, // is_target_loop_entry
);
assert_eq!(result, TailCallKind::ExitJump);
}
@ -127,6 +135,21 @@ mod tests {
true, // has_loop_header_phis
true, // has_boundary
true, // is_target_continuation ← this makes it ExitJump
true, // is_target_loop_entry
);
assert_eq!(result, TailCallKind::ExitJump);
}
#[test]
fn test_classify_inner_step_recursion() {
// Phase 287 P2: inner_step→inner_step should NOT be BackEdge
// even with boundary and header PHIs, because target is not loop entry func
let result = classify_tail_call(
false, // is_entry_func_entry_block (inner_step body block)
true, // has_loop_header_phis
true, // has_boundary
false, // is_target_continuation
false, // is_target_loop_entry ← target is NOT loop entry (inner_step, not loop_step)
);
assert_eq!(result, TailCallKind::ExitJump);
}