fix(joinir): Phase 132 P1 - k_exit continuation classification fix
Problem: k_exit with TailCall(post_k) was classified as skippable continuation,
causing post_k block to become unreachable.
Root cause:
- Continuation classification was name-based ("join_func_2")
- Skippable continuation check didn't examine function body
- k_exit with tail call to post_k had its block skipped during merge
Solution:
- Check function body structure before classifying as skippable
- Non-skippable pattern: continuation with TailCall to another function
- Properly emit tail call as Jump instruction
Changes:
- src/mir/builder/control_flow/joinir/merge/mod.rs:
- Add structural check in is_skippable_continuation()
- Detect TailCall(other_func) pattern
- Skip only if function is truly empty (1 block, Return only)
- src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs:
- Improve k_exit continuation handling
- Properly rewrite TailCall → Jump for post_k
Test results:
- Phase 132 LLVM EXE: PASS (exit code 3)
- Phase 132 VM: PASS
- Phase 131 regression: PASS
- Phase 97 regression: PASS
Related: Phase 132 loop(true) + post-loop VM/LLVM parity
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@ -18,8 +18,103 @@ use crate::mir::builder::joinir_id_remapper::JoinIrIdRemapper;
|
|||||||
use crate::mir::join_ir::lowering::error_tags;
|
use crate::mir::join_ir::lowering::error_tags;
|
||||||
use crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary;
|
use crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary;
|
||||||
use crate::mir::join_ir_vm_bridge::join_func_name;
|
use crate::mir::join_ir_vm_bridge::join_func_name;
|
||||||
use crate::mir::{BasicBlock, BasicBlockId, MirInstruction, MirModule, ValueId};
|
use crate::mir::types::ConstValue;
|
||||||
|
use crate::mir::{BasicBlock, BasicBlockId, MirFunction, MirInstruction, MirModule, MirType, ValueId};
|
||||||
use std::collections::BTreeMap; // Phase 222.5-E: HashMap → BTreeMap for determinism
|
use std::collections::BTreeMap; // Phase 222.5-E: HashMap → BTreeMap for determinism
|
||||||
|
use std::collections::BTreeSet;
|
||||||
|
|
||||||
|
/// Phase 132-R0 Task 3: Structural check for skippable continuation functions
|
||||||
|
///
|
||||||
|
/// A continuation function is skippable if it is a pure exit stub:
|
||||||
|
/// - 1 block only
|
||||||
|
/// - No instructions
|
||||||
|
/// - Return terminator only
|
||||||
|
///
|
||||||
|
/// This is a structural check (no by-name/by-id inference).
|
||||||
|
pub(super) fn is_skippable_continuation(func: &MirFunction) -> bool {
|
||||||
|
if func.blocks.len() != 1 {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
let Some(block) = func.blocks.get(&func.entry_block) else {
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
if !block.instructions.is_empty() {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
matches!(block.terminator, Some(MirInstruction::Return { .. }))
|
||||||
|
}
|
||||||
|
|
||||||
|
fn propagate_value_type_for_inst(
|
||||||
|
builder: &mut crate::mir::builder::MirBuilder,
|
||||||
|
source_func: &MirFunction,
|
||||||
|
old_inst: &MirInstruction,
|
||||||
|
new_inst: &MirInstruction,
|
||||||
|
) {
|
||||||
|
let Some(_) = builder.scope_ctx.current_function else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
|
||||||
|
let (old_dst, new_dst) = match (old_inst, new_inst) {
|
||||||
|
(MirInstruction::Const { dst: o, .. }, MirInstruction::Const { dst: n, .. }) => {
|
||||||
|
(o, n)
|
||||||
|
}
|
||||||
|
(MirInstruction::BinOp { dst: o, .. }, MirInstruction::BinOp { dst: n, .. }) => (o, n),
|
||||||
|
(MirInstruction::UnaryOp { dst: o, .. }, MirInstruction::UnaryOp { dst: n, .. }) => (o, n),
|
||||||
|
(MirInstruction::Compare { dst: o, .. }, MirInstruction::Compare { dst: n, .. }) => (o, n),
|
||||||
|
(MirInstruction::Load { dst: o, .. }, MirInstruction::Load { dst: n, .. }) => (o, n),
|
||||||
|
(MirInstruction::Cast { dst: o, .. }, MirInstruction::Cast { dst: n, .. }) => (o, n),
|
||||||
|
(MirInstruction::TypeOp { dst: o, .. }, MirInstruction::TypeOp { dst: n, .. }) => (o, n),
|
||||||
|
(MirInstruction::ArrayGet { dst: o, .. }, MirInstruction::ArrayGet { dst: n, .. }) => (o, n),
|
||||||
|
(MirInstruction::Copy { dst: o, .. }, MirInstruction::Copy { dst: n, .. }) => (o, n),
|
||||||
|
(MirInstruction::NewBox { dst: o, .. }, MirInstruction::NewBox { dst: n, .. }) => (o, n),
|
||||||
|
(MirInstruction::TypeCheck { dst: o, .. }, MirInstruction::TypeCheck { dst: n, .. }) => (o, n),
|
||||||
|
(MirInstruction::Phi { dst: o, .. }, MirInstruction::Phi { dst: n, .. }) => (o, n),
|
||||||
|
_ => return,
|
||||||
|
};
|
||||||
|
|
||||||
|
// Prefer SSOT from the source MIR (JoinIR→MIR bridge may have hints).
|
||||||
|
if let Some(ty) = source_func.metadata.value_types.get(old_dst).cloned() {
|
||||||
|
builder.type_ctx.value_types.insert(*new_dst, ty);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Fallback inference from the rewritten instruction itself.
|
||||||
|
//
|
||||||
|
// This is important for the LLVM harness: it relies on `dst_type` hints to keep `+`
|
||||||
|
// as numeric add instead of `concat_hh_mixed` when types are otherwise unknown.
|
||||||
|
match new_inst {
|
||||||
|
MirInstruction::Const { dst, value } if dst == new_dst => {
|
||||||
|
let ty = match value {
|
||||||
|
ConstValue::Integer(_) => Some(MirType::Integer),
|
||||||
|
ConstValue::Float(_) => Some(MirType::Float),
|
||||||
|
ConstValue::Bool(_) => Some(MirType::Bool),
|
||||||
|
ConstValue::String(_) => Some(MirType::String),
|
||||||
|
ConstValue::Void => Some(MirType::Void),
|
||||||
|
_ => None,
|
||||||
|
};
|
||||||
|
if let Some(ty) = ty {
|
||||||
|
builder.type_ctx.value_types.insert(*dst, ty);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
MirInstruction::Copy { dst, src } if dst == new_dst => {
|
||||||
|
if let Some(src_ty) = builder.type_ctx.value_types.get(src).cloned() {
|
||||||
|
builder.type_ctx.value_types.insert(*dst, src_ty);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
MirInstruction::BinOp { dst, lhs, rhs, .. } if dst == new_dst => {
|
||||||
|
let lhs_ty = builder.type_ctx.value_types.get(lhs);
|
||||||
|
let rhs_ty = builder.type_ctx.value_types.get(rhs);
|
||||||
|
if matches!(lhs_ty, Some(MirType::Integer)) && matches!(rhs_ty, Some(MirType::Integer))
|
||||||
|
{
|
||||||
|
builder.type_ctx.value_types.insert(*dst, MirType::Integer);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
MirInstruction::Compare { dst, .. } if dst == new_dst => {
|
||||||
|
builder.type_ctx.value_types.insert(*dst, MirType::Bool);
|
||||||
|
}
|
||||||
|
_ => {}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Phase 4: Merge ALL functions and rewrite instructions
|
/// Phase 4: Merge ALL functions and rewrite instructions
|
||||||
///
|
///
|
||||||
@ -61,8 +156,31 @@ pub(super) fn merge_and_rewrite(
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
// Phase 131 Task 2: Create tail call lowering policy for k_exit normalization
|
let continuation_candidates: BTreeSet<String> = boundary
|
||||||
let tail_call_policy = TailCallLoweringPolicyBox::new();
|
.map(|b| {
|
||||||
|
b.continuation_func_ids
|
||||||
|
.iter()
|
||||||
|
.copied()
|
||||||
|
.map(join_func_name)
|
||||||
|
.collect()
|
||||||
|
})
|
||||||
|
.unwrap_or_default();
|
||||||
|
|
||||||
|
let skippable_continuation_func_names: BTreeSet<String> = mir_module
|
||||||
|
.functions
|
||||||
|
.iter()
|
||||||
|
.filter_map(|(func_name, func)| {
|
||||||
|
if continuation_candidates.contains(func_name) && is_skippable_continuation(func) {
|
||||||
|
Some(func_name.clone())
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
|
||||||
|
// Phase 132 P1: Tail call lowering is driven by the continuation contract (SSOT)
|
||||||
|
// Only structurally-skippable continuation functions are lowered to exit jumps.
|
||||||
|
let tail_call_policy = TailCallLoweringPolicyBox::new(skippable_continuation_func_names.clone());
|
||||||
|
|
||||||
// Phase 177-3: exit_block_id is now passed in from block_allocator
|
// Phase 177-3: exit_block_id is now passed in from block_allocator
|
||||||
log!(
|
log!(
|
||||||
@ -109,36 +227,36 @@ pub(super) fn merge_and_rewrite(
|
|||||||
functions_merge.sort_by_key(|(name, _)| name.as_str());
|
functions_merge.sort_by_key(|(name, _)| name.as_str());
|
||||||
|
|
||||||
let entry_func_name = functions_merge.first().map(|(name, _)| name.as_str());
|
let entry_func_name = functions_merge.first().map(|(name, _)| name.as_str());
|
||||||
// Phase 131 Task 2: k_exit detection now handled by TailCallLoweringPolicyBox
|
|
||||||
let k_exit_func_name = join_func_name(crate::mir::join_ir::JoinFuncId::new(2));
|
|
||||||
|
|
||||||
for (func_name, func) in functions_merge {
|
for (func_name, func) in functions_merge {
|
||||||
// Phase 33-15: Identify continuation functions (k_exit, etc.)
|
// Phase 33-15: Identify continuation functions (k_exit, etc.)
|
||||||
// Continuation functions receive values from Jump args, not as independent sources
|
// Continuation functions receive values from Jump args, not as independent sources
|
||||||
// We should NOT collect their Return values for exit_phi_inputs
|
// We should NOT collect their Return values for exit_phi_inputs
|
||||||
// Phase 131 Task 2: k_exit function name still used for continuation function detection
|
let is_continuation_candidate = continuation_candidates.contains(func_name);
|
||||||
let is_continuation_func = func_name == &k_exit_func_name;
|
let is_skippable_continuation = skippable_continuation_func_names.contains(func_name);
|
||||||
|
|
||||||
if debug {
|
if debug {
|
||||||
log!(
|
log!(
|
||||||
true,
|
true,
|
||||||
"[cf_loop/joinir] Merging function '{}' with {} blocks, entry={:?} (is_continuation={})",
|
"[cf_loop/joinir] Merging function '{}' with {} blocks, entry={:?} (continuation_candidate={}, skippable_continuation={})",
|
||||||
func_name,
|
func_name,
|
||||||
func.blocks.len(),
|
func.blocks.len(),
|
||||||
func.entry_block,
|
func.entry_block,
|
||||||
is_continuation_func
|
is_continuation_candidate,
|
||||||
|
is_skippable_continuation
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Phase 33-15: Skip continuation function blocks entirely
|
// Phase 132 P1: Skip only *structurally skippable* continuation functions.
|
||||||
// Continuation functions (k_exit) are just exit points; their parameters are
|
//
|
||||||
// passed via Jump args which become Return values in other functions.
|
// Continuation functions can be real code (e.g. k_exit tailcalls post_k).
|
||||||
// Processing continuation functions would add undefined ValueIds to PHI.
|
// Merge must follow the explicit continuation contract and then decide
|
||||||
if is_continuation_func {
|
// skippability by structure only.
|
||||||
|
if is_skippable_continuation {
|
||||||
if debug {
|
if debug {
|
||||||
log!(
|
log!(
|
||||||
true,
|
true,
|
||||||
"[cf_loop/joinir] Phase 33-15: Skipping continuation function '{}' blocks",
|
"[cf_loop/joinir] Phase 132 P1: Skipping skippable continuation function '{}' blocks",
|
||||||
func_name
|
func_name
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@ -401,6 +519,7 @@ pub(super) fn merge_and_rewrite(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
propagate_value_type_for_inst(builder, func, inst, &remapped_with_blocks);
|
||||||
new_block.instructions.push(remapped_with_blocks);
|
new_block.instructions.push(remapped_with_blocks);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -28,6 +28,9 @@ mod tail_call_classifier;
|
|||||||
mod tail_call_lowering_policy; // Phase 131 Task 2: k_exit exit edge normalization
|
mod tail_call_lowering_policy; // Phase 131 Task 2: k_exit exit edge normalization
|
||||||
mod value_collector;
|
mod value_collector;
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests; // Phase 132-R0 Task 3: Continuation contract tests
|
||||||
|
|
||||||
// Phase 33-17: Re-export for use by other modules
|
// Phase 33-17: Re-export for use by other modules
|
||||||
pub use loop_header_phi_builder::LoopHeaderPhiBuilder;
|
pub use loop_header_phi_builder::LoopHeaderPhiBuilder;
|
||||||
pub use loop_header_phi_info::LoopHeaderPhiInfo;
|
pub use loop_header_phi_info::LoopHeaderPhiInfo;
|
||||||
|
|||||||
@ -395,7 +395,7 @@ impl MirBuilder {
|
|||||||
"Pattern router found no match, trying legacy whitelist",
|
"Pattern router found no match, trying legacy whitelist",
|
||||||
);
|
);
|
||||||
|
|
||||||
// Delegate to legacy binding path (routing_legacy_binding.rs)
|
// Phase 132-R0 Task 4: Delegate to legacy binding path (legacy/routing_legacy_binding.rs)
|
||||||
self.cf_loop_joinir_legacy_binding(condition, body, func_name, debug)
|
self.cf_loop_joinir_legacy_binding(condition, body, func_name, debug)
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -495,6 +495,7 @@ impl MirBuilder {
|
|||||||
exit_bindings,
|
exit_bindings,
|
||||||
);
|
);
|
||||||
boundary.exit_reconnect_mode = ExitReconnectMode::DirectValue; // Phase 131 P1.5: No PHI
|
boundary.exit_reconnect_mode = ExitReconnectMode::DirectValue; // Phase 131 P1.5: No PHI
|
||||||
|
boundary.continuation_func_ids = join_meta.continuation_funcs.clone();
|
||||||
|
|
||||||
// Merge with boundary - this will populate MergeResult.remapped_exit_values
|
// Merge with boundary - this will populate MergeResult.remapped_exit_values
|
||||||
use crate::mir::builder::control_flow::joinir::merge;
|
use crate::mir::builder::control_flow::joinir::merge;
|
||||||
|
|||||||
@ -7,7 +7,8 @@
|
|||||||
|
|
||||||
use crate::ast::Span;
|
use crate::ast::Span;
|
||||||
use crate::mir::join_ir::{JoinInst, MirLikeInst};
|
use crate::mir::join_ir::{JoinInst, MirLikeInst};
|
||||||
use crate::mir::{BasicBlockId, EffectMask, MirFunction, MirInstruction, ValueId};
|
use crate::mir::{BasicBlockId, EffectMask, MirFunction, MirInstruction, MirType, ValueId};
|
||||||
|
use crate::mir::types::ConstValue;
|
||||||
|
|
||||||
use super::{convert_mir_like_inst, join_func_name, JoinIrVmBridgeError};
|
use super::{convert_mir_like_inst, join_func_name, JoinIrVmBridgeError};
|
||||||
|
|
||||||
@ -71,6 +72,7 @@ impl JoinIrBlockConverter {
|
|||||||
mir_like
|
mir_like
|
||||||
));
|
));
|
||||||
let mir_inst = convert_mir_like_inst(mir_like)?;
|
let mir_inst = convert_mir_like_inst(mir_like)?;
|
||||||
|
Self::annotate_value_types_for_inst(mir_func, &mir_inst);
|
||||||
self.current_instructions.push(mir_inst);
|
self.current_instructions.push(mir_inst);
|
||||||
}
|
}
|
||||||
JoinInst::MethodCall {
|
JoinInst::MethodCall {
|
||||||
@ -154,6 +156,48 @@ impl JoinIrBlockConverter {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn annotate_value_types_for_inst(mir_func: &mut MirFunction, inst: &MirInstruction) {
|
||||||
|
match inst {
|
||||||
|
MirInstruction::Const { dst, value } => {
|
||||||
|
let ty = match value {
|
||||||
|
ConstValue::Integer(_) => Some(MirType::Integer),
|
||||||
|
ConstValue::Float(_) => Some(MirType::Float),
|
||||||
|
ConstValue::Bool(_) => Some(MirType::Bool),
|
||||||
|
ConstValue::String(_) => Some(MirType::String),
|
||||||
|
ConstValue::Void => Some(MirType::Void),
|
||||||
|
_ => None,
|
||||||
|
};
|
||||||
|
if let Some(ty) = ty {
|
||||||
|
mir_func.metadata.value_types.insert(*dst, ty);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
MirInstruction::BinOp { dst, lhs, rhs, .. } => {
|
||||||
|
// Conservative typing: only mark integer when both operands are already known integers.
|
||||||
|
//
|
||||||
|
// This avoids forcing numeric semantics for `+` in cases where the operands are not
|
||||||
|
// known to be integers (e.g. string concatenation patterns).
|
||||||
|
let lhs_ty = mir_func.metadata.value_types.get(lhs);
|
||||||
|
let rhs_ty = mir_func.metadata.value_types.get(rhs);
|
||||||
|
if matches!(lhs_ty, Some(MirType::Integer)) && matches!(rhs_ty, Some(MirType::Integer))
|
||||||
|
{
|
||||||
|
mir_func.metadata.value_types.insert(*dst, MirType::Integer);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
MirInstruction::Compare { dst, .. } => {
|
||||||
|
mir_func.metadata.value_types.insert(*dst, MirType::Bool);
|
||||||
|
}
|
||||||
|
MirInstruction::TypeCheck { dst, .. } => {
|
||||||
|
mir_func.metadata.value_types.insert(*dst, MirType::Bool);
|
||||||
|
}
|
||||||
|
MirInstruction::Phi { dst, type_hint, .. } => {
|
||||||
|
if let Some(ty) = type_hint.clone() {
|
||||||
|
mir_func.metadata.value_types.insert(*dst, ty);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
_ => {}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// === Instruction Handlers ===
|
// === Instruction Handlers ===
|
||||||
|
|
||||||
fn handle_method_call(
|
fn handle_method_call(
|
||||||
|
|||||||
Reference in New Issue
Block a user