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

wip(joinir): Phase 256 P1.10 - classify_tail_call continuation support

Browse Source 
## 修正内容
- classify_tail_call に is_target_continuation パラメータ追加
- k_exit 呼び出しを ExitJump として分類（BackEdge に誤分類されるのを防止）
- entry_func_name の決定ロジックを修正（loop_step を正しく識別）
- Continuation param bindings の Copy 生成ロジック追加

## 残存問題
bb8/bb9 の instructions が 0 になる問題
- add_block 時は 4 instructions ある
- printer 時は instructions=0, spans=4
- 原因調査中

🔍 WIP - ChatGPT レビュー用

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
tomoaki
2025-12-20 10:25:13 +09:00
parent cb8a6984d4
commit 2c4268b691
4 changed files with 258 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@ -210,6 +210,10 @@ pub(super) fn merge_and_rewrite(
function_entry_map.insert(func_name.clone(), entry_block_new);
}
// Phase 256 P1.10: Pre-pass removed - param mappings are set up during block processing
// The issue was that pre-pass used stale remapper values before Phase 33-21 updates.
// Instead, we now generate Copies for non-carrier params in the main block processing loop.
// DETERMINISM FIX: Sort functions by name to ensure consistent iteration order
if debug {
log!(
@ -222,7 +226,22 @@ pub(super) fn merge_and_rewrite(
let mut functions_merge: Vec<_> = mir_module.functions.iter().collect();
functions_merge.sort_by_key(|(name, _)| name.as_str());
let entry_func_name = functions_merge.first().map(|(name, _)| name.as_str());
// Phase 256 P1.10: Determine actual entry function (loop header)
//
// The entry function is the one that:
// - Is NOT a continuation function (k_exit, etc.)
// - Is NOT "main" (the trampoline that calls the loop entry)
//
// This is the function that receives boundary inputs and is the loop header.
let entry_func_name = functions_merge
.iter()
.find(|(name, _)| {
let name_str = name.as_str();
let is_continuation = continuation_candidates.contains(*name);
let is_main = name_str == crate::mir::join_ir::lowering::canonical_names::MAIN;
!is_continuation && !is_main
})
.map(|(name, _)| name.as_str());
for (func_name, func) in functions_merge {
// Phase 33-15: Identify continuation functions (k_exit, etc.)
@ -301,6 +320,23 @@ pub(super) fn merge_and_rewrite(
entry_func_name == Some(func_name.as_str()) && *old_block_id == func.entry_block;
// DEBUG: Print block being processed
// Phase 256 P1.10: Always log block mapping for debugging
log!(
true,
"[cf_loop/joinir] Phase 256 P1.10: Block mapping: func='{}' old={:?} → new={:?} (inst_count={})",
func_name, old_block_id, new_block_id, old_block.instructions.len()
);
// Phase 256 P1.10 DEBUG: Log first instruction for k_exit to trace content placement
if func_name == "k_exit" {
if let Some(first_inst) = old_block.instructions.first() {
log!(
true,
"[cf_loop/joinir] Phase 256 P1.10 DEBUG: k_exit first instruction: {:?}",
first_inst
);
}
}
if debug {
log!(
true,
@ -552,8 +588,40 @@ pub(super) fn merge_and_rewrite(
}
}
if let Some(target_func_name) = target_func_name {
if let Some(target_params) = function_params.get(&target_func_name) {
// Phase 256 P1.10: Define is_target_continuation for classify_tail_call
// This must be defined before the inner if lets so it's in scope for classify_tail_call
let is_target_continuation = target_func_name
.as_ref()
.map(|name| continuation_candidates.contains(name))
.unwrap_or(false);
if let Some(ref target_func_name) = target_func_name {
if let Some(target_params) = function_params.get(target_func_name) {
// Phase 256 P1.10: Detect call type for param binding strategy
// - Recursive call (loop_step → loop_step): Skip all param bindings (PHI handles via latch edges)
// - Exit call (loop_step → k_exit): Skip all param bindings (exit PHI handles via exit edges)
// - Header entry (main → loop_step at header): Skip all (PHI handles via entry edges)
let is_recursive_call = target_func_name == func_name;
// Phase 256 P1.10: Detect if target is the loop entry function
// When calling the loop entry function, header PHIs will define the carriers.
// We should skip param bindings in this case.
let is_target_loop_entry = entry_func_name == Some(target_func_name.as_str());
// Phase 256 P1.10: Detect if target is a continuation (k_exit)
// k_exit params are exit binding values, which are remapped to exit PHI dsts.
// Generating Copies for k_exit would create multiple definitions (Copy + PHI).
// Use continuation_candidates (not skippable_continuation_func_names) because
// k_exit may have instructions (non-skippable) but still needs param skip.
let is_target_continuation = continuation_candidates.contains(target_func_name);
// Phase 256 P1.10 DEBUG: Always log call type detection
log!(
true,
"[cf_loop/joinir] Phase 256 P1.10: Tail call from '{}' to '{}' (is_recursive={}, is_loop_entry={}, is_target_loop_entry={}, is_target_continuation={})",
func_name, target_func_name, is_recursive_call, is_loop_entry_point, is_target_loop_entry, is_target_continuation
);
// Phase 33-21: Skip parameter binding in header block
//
// The header block (loop entry point) has PHIs that define carriers.
@ -572,6 +640,58 @@ pub(super) fn merge_and_rewrite(
"[cf_loop/joinir] Phase 33-21: Skip param bindings in header block (PHIs define carriers)"
);
}
} else if is_recursive_call || is_target_loop_entry {
// Phase 256 P1.10: Skip param bindings for:
// - Recursive call (loop_step → loop_step): latch edge
// - Entry call (main → loop_step): entry edge
//
// Header PHIs receive values from these edges via separate mechanism.
// Generating Copies here would cause multiple definitions.
//
// Update remapper mappings for any further instructions.
for (i, arg_val_remapped) in args.iter().enumerate() {
if i < target_params.len() {
let param_val_original = target_params[i];
remapper.set_value(param_val_original, *arg_val_remapped);
}
}
log!(
true,
"[cf_loop/joinir] Phase 256 P1.10: Skip Copy bindings for {} call to '{}' (remapper updated)",
if is_recursive_call { "recursive" } else { "entry" },
target_func_name
);
} else if is_target_continuation {
// Phase 256 P1.10: Continuation call (loop_step → k_exit)
// k_exit body uses its params to compute return values.
// We must Copy call args to ORIGINAL params (not remapped).
// Params are excluded from remapping (Phase 256 P1.10 exit_binding skip),
// so using original params avoids conflict with exit PHI.
log!(
true,
"[cf_loop/joinir] Phase 256 P1.10 DEBUG: Before continuation Copies, new_block {:?} has {} instructions",
new_block_id, new_block.instructions.len()
);
for (i, arg_val_remapped) in args.iter().enumerate() {
if i < target_params.len() {
let param_val_original = target_params[i];
// Use original param as dst - it won't be remapped
new_block.instructions.push(MirInstruction::Copy {
dst: param_val_original,
src: *arg_val_remapped,
});
log!(
true,
"[cf_loop/joinir] Phase 256 P1.10: Continuation param binding: {:?} = copy {:?}",
param_val_original, arg_val_remapped
);
}
}
log!(
true,
"[cf_loop/joinir] Phase 256 P1.10 DEBUG: After continuation Copies, new_block {:?} has {} instructions",
new_block_id, new_block.instructions.len()
);
} else {
// Insert Copy instructions for parameter binding
// Phase 131-6 FIX: Skip Copy when dst is a header PHI destination
@ -579,40 +699,49 @@ pub(super) fn merge_and_rewrite(
for (i, arg_val_remapped) in args.iter().enumerate() {
if i < target_params.len() {
let param_val_original = target_params[i];
if let Some(param_val_remapped) =
remapper.get_value(param_val_original)
{
// Phase 131-6: Check if this would overwrite a header PHI dst
let is_header_phi_dst = loop_header_phi_info
.carrier_phis
.values()
.any(|entry| entry.phi_dst == param_val_remapped);
// Phase 256 P1.10: Use original param if not remapped
// Params are excluded from used_values to prevent undefined remaps,
// so we use the original param ValueId as the Copy dst.
let param_remap_result = remapper.get_value(param_val_original);
let param_val_dst = param_remap_result
.unwrap_or(param_val_original);
if is_header_phi_dst {
if debug {
log!(
true,
"[cf_loop/joinir] Phase 131-6: Skip param binding to PHI dst {:?} (PHI receives value via incoming edge)",
param_val_remapped
);
}
// Skip - PHI will receive this value as incoming edge
continue;
}
// Phase 256 P1.10 DEBUG: Log remap decision
log!(
true,
"[cf_loop/joinir] Phase 256 P1.10 DEBUG: Param[{}] original={:?} remap_result={:?} final_dst={:?}",
i, param_val_original, param_remap_result, param_val_dst
);
new_block.instructions.push(MirInstruction::Copy {
dst: param_val_remapped,
src: *arg_val_remapped,
});
// Phase 131-6: Check if this would overwrite a header PHI dst
let is_header_phi_dst = loop_header_phi_info
.carrier_phis
.values()
.any(|entry| entry.phi_dst == param_val_dst);
if is_header_phi_dst {
if debug {
log!(
true,
"[cf_loop/joinir] Param binding: arg {:?} → param {:?}",
arg_val_remapped, param_val_remapped
"[cf_loop/joinir] Phase 131-6: Skip param binding to PHI dst {:?} (PHI receives value via incoming edge)",
param_val_dst
);
}
// Skip - PHI will receive this value as incoming edge
continue;
}
// Phase 256 P1.10 DEBUG: Always log Copy generation
log!(
true,
"[cf_loop/joinir] Phase 256 P1.10 DEBUG: Generating Copy: {:?} = copy {:?} (func='{}', target='{}')",
param_val_dst, arg_val_remapped, func_name, target_func_name
);
new_block.instructions.push(MirInstruction::Copy {
dst: param_val_dst,
src: *arg_val_remapped,
});
}
}
}
@ -726,10 +855,13 @@ pub(super) fn merge_and_rewrite(
// - LoopEntry: First entry (entry function's entry block) → no redirect
// - BackEdge: Loop continuation (other blocks) → redirect to header PHI
// - ExitJump: Exit to continuation → handled by Return conversion
// Phase 256 P1.10: Add is_target_continuation to prevent k_exit calls
// from being redirected to header block.
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,
);
let actual_target = match tail_call_kind {
@ -1003,6 +1135,17 @@ pub(super) fn merge_and_rewrite(
// Add block to current function
if let Some(ref mut current_func) = builder.scope_ctx.current_function {
// Phase 256 P1.10 DEBUG: Log block content before adding (for blocks with multiple instructions)
if new_block.instructions.len() >= 4 {
log!(
true,
"[cf_loop/joinir] Phase 256 P1.10 DEBUG: Adding block {:?} with {} instructions to function",
new_block_id, new_block.instructions.len()
);
for (idx, inst) in new_block.instructions.iter().enumerate() {
log!(true, "[cf_loop/joinir] Phase 256 P1.10 DEBUG: [{}] {:?}", idx, inst);
}
}
current_func.add_block(new_block);
}
}

52
src/mir/builder/control_flow/joinir/merge/mod.rs
View File

@ -230,16 +230,33 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
used_values.insert(binding.join_value);
}
// Phase 172-3: Add exit_bindings' join_exit_values to used_values for remapping
// Phase 172-3 + Phase 256 P1.10: Add exit_bindings' join_exit_values to used_values
// UNLESS they are function params. Params should NOT be remapped (they're defined
// by call site Copies and used directly in k_exit body).
let all_params: std::collections::HashSet<ValueId> = function_params
.values()
.flat_map(|params| params.iter().copied())
.collect();
for binding in &boundary.exit_bindings {
trace.stderr_if(
&format!(
"[cf_loop/joinir] Phase 172-3: Adding exit binding '{}' JoinIR {:?} to used_values",
binding.carrier_name, binding.join_exit_value
),
debug,
);
used_values.insert(binding.join_exit_value);
if all_params.contains(&binding.join_exit_value) {
trace.stderr_if(
&format!(
"[cf_loop/joinir] Phase 256 P1.10: Skipping exit binding '{}' (JoinIR {:?} is a param)",
binding.carrier_name, binding.join_exit_value
),
debug,
);
} else {
trace.stderr_if(
&format!(
"[cf_loop/joinir] Phase 172-3: Adding exit binding '{}' JoinIR {:?} to used_values",
binding.carrier_name, binding.join_exit_value
),
debug,
);
used_values.insert(binding.join_exit_value);
}
}
}
@ -253,10 +270,17 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
let mut loop_header_phi_info = if let Some(boundary) = boundary {
if let Some(loop_var_name) = &boundary.loop_var_name {
// Get entry function and block for building PHI info
// Phase 256 P1.10: Find the actual entry function (loop header)
// The entry function is NOT a continuation (k_exit) and NOT "main".
let (entry_func_name, entry_func) = mir_module
.functions
.iter()
.next()
.find(|(name, _)| {
let is_continuation = boundary.continuation_func_ids.contains(*name);
let is_main = *name == crate::mir::join_ir::lowering::canonical_names::MAIN;
!is_continuation && !is_main
})
.or_else(|| mir_module.functions.iter().next())
.ok_or("JoinIR module has no functions (Phase 201-A)")?;
let entry_block_remapped = remapper
.get_block(entry_func_name, entry_func.entry_block)
@ -310,6 +334,14 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks(
.collect()
};
// Phase 256 P1.10: Always log entry function determination
trace.stderr_if(
&format!(
"[cf_loop/joinir] Phase 256 P1.10: Entry function='{}', entry_block_remapped={:?}",
entry_func_name, entry_block_remapped
),
true, // Always log for debugging
);
trace.stderr_if(
&format!(
"[cf_loop/joinir] Phase 201-A: Pre-building header PHIs for loop_var='{}' at {:?}",

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

@ -37,6 +37,7 @@ pub enum TailCallKind {
/// * `is_entry_func_entry_block` - True if this is the first function's first block (loop entry point)
/// * `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)
///
/// # Returns
/// The classification of this tail call
@ -44,7 +45,15 @@ pub fn classify_tail_call(
is_entry_func_entry_block: bool,
has_loop_header_phis: bool,
has_boundary: bool,
is_target_continuation: bool,
) -> TailCallKind {
// Phase 256 P1.10: Continuation calls (k_exit) are always ExitJump
// They should NOT be redirected to the header block.
// k_exit body needs to execute before exiting.
if is_target_continuation {
return TailCallKind::ExitJump;
}
// Entry function's entry block is the loop entry point
// It already IS at the header, so no redirection needed
if is_entry_func_entry_block {
@ -68,9 +77,10 @@ mod tests {
#[test]
fn test_classify_loop_entry() {
let result = classify_tail_call(
true, // is_entry_func_entry_block
true, // has_loop_header_phis
true, // has_boundary
true, // is_entry_func_entry_block
true, // has_loop_header_phis
true, // has_boundary
false, // is_target_continuation
);
assert_eq!(result, TailCallKind::LoopEntry);
}
@ -81,6 +91,7 @@ mod tests {
false, // is_entry_func_entry_block (not entry block)
true, // has_loop_header_phis
true, // has_boundary
false, // is_target_continuation
);
assert_eq!(result, TailCallKind::BackEdge);
}
@ -91,6 +102,7 @@ mod tests {
false, // is_entry_func_entry_block
false, // has_loop_header_phis (no header PHIs)
true, // has_boundary
false, // is_target_continuation
);
assert_eq!(result, TailCallKind::ExitJump);
}
@ -101,6 +113,20 @@ mod tests {
false, // is_entry_func_entry_block
true, // has_loop_header_phis
false, // has_boundary (no boundary → exit)
false, // is_target_continuation
);
assert_eq!(result, TailCallKind::ExitJump);
}
#[test]
fn test_classify_continuation_target() {
// Phase 256 P1.10: Continuation calls (k_exit) are always ExitJump
// even when they would otherwise be classified as BackEdge
let result = classify_tail_call(
false, // is_entry_func_entry_block
true, // has_loop_header_phis
true, // has_boundary
true, // is_target_continuation ← this makes it ExitJump
);
assert_eq!(result, TailCallKind::ExitJump);
}

24
src/mir/builder/control_flow/joinir/merge/value_collector.rs
View File

@ -52,6 +52,15 @@ pub(super) fn collect_values(
// Phase 188-Impl-3: Collect function parameters for tail call conversion
function_params.insert(func_name.clone(), func.params.clone());
// Phase 256 P1.10 DEBUG: Always log function params for debugging
trace.stderr_if(
&format!(
"[cf_loop/joinir] Phase 256 P1.10 DEBUG: Function '{}' params: {:?}",
func_name, func.params
),
true,
);
for block in func.blocks.values() {
// Phase 189: Use remapper to collect values
let block_values = remapper.collect_values_in_block(block);
@ -81,7 +90,7 @@ pub(super) fn collect_values(
}
}
// Phase 33-15: DO NOT collect parameter ValueIds into used_values
// Phase 33-15 / Phase 256 P1.10: DO NOT remap parameter ValueIds
//
// Reasoning: Parameters are implicitly defined at function entry in JoinIR.
// When inlined into host MIR, if parameters are remapped, the remapped ValueIds
@ -92,13 +101,12 @@ pub(super) fn collect_values(
// 1. BoundaryInjector Copy (for entry function: host_input → join_param)
// 2. Tail call Copy (for recursive calls: call_arg → param)
//
// We still store parameters in function_params for tail call handling,
// but don't add them to used_values so they won't be remapped.
//
// OLD CODE (removed):
// for param in &func.params {
// used_values.insert(*param);
// }
// Phase 256 P1.10: We must REMOVE params from used_values after block collection,
// because collect_values_in_block() adds all operand ValueIds including params.
// Without this removal, params get remapped and their remapped values are undefined.
for param in &func.params {
used_values.remove(param);
}
}
if debug {