From 1080dee58fd6a18b088df236688f4f8bc088603e Mon Sep 17 00:00:00 2001 From: nyash-codex Date: Thu, 18 Dec 2025 03:43:00 +0900 Subject: [PATCH] fix(joinir): Phase 118 Pattern3 exit carrier PHI SSOT --- .../joinir/merge/contract_checks.rs | 43 +++++- .../joinir/merge/instruction_rewriter.rs | 137 +++++++++--------- .../builder/control_flow/joinir/merge/mod.rs | 43 +++++- .../lowering/loop_with_if_phi_if_sum.rs | 72 ++------- 4 files changed, 161 insertions(+), 134 deletions(-) diff --git a/src/mir/builder/control_flow/joinir/merge/contract_checks.rs b/src/mir/builder/control_flow/joinir/merge/contract_checks.rs index 4899af7f..d4a78ac9 100644 --- a/src/mir/builder/control_flow/joinir/merge/contract_checks.rs +++ b/src/mir/builder/control_flow/joinir/merge/contract_checks.rs @@ -1,9 +1,48 @@ -use super::LoopHeaderPhiInfo; use crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary; +use crate::mir::ValueId; +use std::collections::BTreeMap; + +#[cfg(debug_assertions)] +use super::LoopHeaderPhiInfo; +#[cfg(debug_assertions)] use crate::mir::join_ir::lowering::join_value_space::{LOCAL_MAX, PARAM_MAX, PARAM_MIN}; -use crate::mir::{BasicBlockId, MirFunction, MirInstruction, ValueId}; +#[cfg(debug_assertions)] +use crate::mir::{BasicBlockId, MirFunction, MirInstruction}; +#[cfg(debug_assertions)] use std::collections::HashMap; +/// Phase 118 P2: Contract check (Fail-Fast) - exit_bindings carriers must have exit PHI dsts. +/// +/// This prevents latent "Carrier '' not found in carrier_phis" failures later in +/// ExprResultResolver and ExitLine reconnection. +pub(super) fn verify_exit_bindings_have_exit_phis( + boundary: &JoinInlineBoundary, + exit_carrier_phis: &BTreeMap, +) -> Result<(), String> { + use crate::mir::join_ir::lowering::carrier_info::CarrierRole; + use crate::mir::join_ir::lowering::error_tags; + + for binding in &boundary.exit_bindings { + if binding.role == CarrierRole::ConditionOnly { + continue; + } + if exit_carrier_phis.contains_key(&binding.carrier_name) { + continue; + } + + return Err(error_tags::freeze_with_hint( + "phase118/exit_phi/missing_carrier_phi", + &format!( + "exit_bindings carrier '{}' is missing from exit_carrier_phis", + binding.carrier_name + ), + "exit_bindings carriers must be included in exit_phi_builder inputs; check carrier_inputs derivation from jump_args", + )); + } + + Ok(()) +} + #[cfg(debug_assertions)] pub(super) fn verify_loop_header_phis( func: &MirFunction, diff --git a/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs b/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs index 1c545336..61765018 100644 --- a/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs +++ b/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs @@ -650,12 +650,46 @@ pub(super) fn merge_and_rewrite( // Phase 246-EX: Collect exit values from jump_args if let Some(b) = boundary { - if let Some(ref carrier_info) = b.carrier_info { - let expected_args = carrier_info.carriers.len() + 1; // loop_var + carriers - if remapped_args.len() < expected_args { + // ExprResult collection: first jump arg is preserved as Return value by the converter, + // and is the SSOT for exit_phi_inputs in this merge phase. + if let Some(&first_exit) = remapped_args.first() { + exit_phi_inputs.push((new_block_id, first_exit)); + log!( + verbose, + "[DEBUG-177] Phase 246-EX: exit_phi_inputs from jump_args[0]: ({:?}, {:?})", + new_block_id, first_exit + ); + } + + // Phase 118 P2: carrier_inputs SSOT = boundary.exit_bindings + // + // Contract: + // - Every LoopState carrier in exit_bindings must have an exit PHI input. + // - jump_args order is assumed to match exit_bindings order, with at most one + // leading extra slot (legacy layouts). + // + // This avoids Pattern3-specific assumptions such as "jump_args[0] is loop_var". + use crate::mir::join_ir::lowering::carrier_info::CarrierRole; + let exit_phi_bindings: Vec<_> = b + .exit_bindings + .iter() + .filter(|eb| eb.role != CarrierRole::ConditionOnly) + .collect(); + + if !exit_phi_bindings.is_empty() { + let offset = if remapped_args.len() + == exit_phi_bindings.len() + { + 0usize + } else if remapped_args.len() + == exit_phi_bindings.len() + 1 + { + 1usize + } else if remapped_args.len() < exit_phi_bindings.len() + { let msg = format!( - "[joinir/exit-line] jump_args length mismatch: expected at least {} (loop_var + carriers) but got {} in block {:?}", - expected_args, + "[joinir/exit-line] jump_args too short: need {} carrier args (from exit_bindings) but got {} in block {:?}", + exit_phi_bindings.len(), remapped_args.len(), old_block.id ); @@ -664,91 +698,56 @@ pub(super) fn merge_and_rewrite( } else { log!(verbose, "[DEBUG-177] {}", msg); } - } - } - - // First arg is the loop variable (expr_result) - if let Some(&loop_var_exit) = remapped_args.first() { - exit_phi_inputs.push((new_block_id, loop_var_exit)); - log!( - verbose, - "[DEBUG-177] Phase 246-EX: exit_phi_inputs from jump_args[0]: ({:?}, {:?})", - new_block_id, loop_var_exit - ); - - // Phase 246-EX-P5: Also add loop_var to carrier_inputs if it's a named variable - // (Pattern 5 case: counter is both loop_var AND a carrier) - if let Some(ref loop_var_name) = b.loop_var_name { - carrier_inputs - .entry(loop_var_name.clone()) - .or_insert_with(Vec::new) - .push((new_block_id, loop_var_exit)); - log!( - verbose, - "[DEBUG-177] Phase 246-EX-P5: Added loop_var '{}' to carrier_inputs: ({:?}, {:?})", - loop_var_name, new_block_id, loop_var_exit + 0usize + } else { + let msg = format!( + "[joinir/exit-line] jump_args length mismatch: expected {} or {} (exit_bindings carriers ±1) but got {} in block {:?}", + exit_phi_bindings.len(), + exit_phi_bindings.len() + 1, + remapped_args.len(), + old_block.id ); - } - } - - // Phase 246-EX-FIX: jump_args are in carrier_info.carriers order, not exit_bindings order! - // - // jump_args layout: [loop_var, carrier1, carrier2, ...] - // where carriers follow carrier_info.carriers order (NOT exit_bindings order) - // - // We need to: - // 1. Iterate through carrier_info.carriers (to get the right index) - // 2. Skip ConditionOnly carriers (they don't participate in exit PHI) - // 3. Map the jump_args index to the carrier name - if let Some(ref carrier_info) = b.carrier_info { - for (carrier_idx, carrier) in - carrier_info.carriers.iter().enumerate() - { - // Phase 227: Skip ConditionOnly carriers - if carrier.role == crate::mir::join_ir::lowering::carrier_info::CarrierRole::ConditionOnly { - log!( - verbose, - "[DEBUG-177] Phase 227: Skipping ConditionOnly carrier '{}' from exit PHI", - carrier.name - ); - continue; + if strict_exit { + return Err(msg); + } else { + log!(verbose, "[DEBUG-177] {}", msg); } + 0usize + }; - // jump_args[0] = loop_var, jump_args[1..] = carriers - let jump_args_idx = carrier_idx + 1; + for (binding_idx, binding) in + exit_phi_bindings.iter().enumerate() + { + let jump_args_idx = offset + binding_idx; if let Some(&carrier_exit) = remapped_args.get(jump_args_idx) { carrier_inputs - .entry(carrier.name.clone()) + .entry(binding.carrier_name.clone()) .or_insert_with(Vec::new) .push((new_block_id, carrier_exit)); log!( verbose, - "[DEBUG-177] Phase 246-EX-FIX: Collecting carrier '{}': from {:?} using jump_args[{}] = {:?}", - carrier.name, new_block_id, jump_args_idx, carrier_exit + "[DEBUG-177] Phase 118: Collecting carrier '{}': from {:?} using jump_args[{}] = {:?}", + binding.carrier_name, + new_block_id, + jump_args_idx, + carrier_exit ); } else { let msg = format!( - "[joinir/exit-line] Missing jump_args entry for carrier '{}' at index {} in block {:?}", - carrier.name, jump_args_idx, old_block.id + "[joinir/exit-line] Missing jump_args entry for exit_binding carrier '{}' at index {} in block {:?}", + binding.carrier_name, + jump_args_idx, + old_block.id ); if strict_exit { return Err(msg); } else { - log!( - verbose, - "[DEBUG-177] Phase 246-EX WARNING: No jump_args entry for carrier '{}' at index {}", - carrier.name, jump_args_idx - ); + log!(verbose, "[DEBUG-177] {}", msg); } } } - } else { - log!( - verbose, - "[DEBUG-177] Phase 246-EX WARNING: No carrier_info in boundary!" - ); } } } else { diff --git a/src/mir/builder/control_flow/joinir/merge/mod.rs b/src/mir/builder/control_flow/joinir/merge/mod.rs index df8a83bc..df179340 100644 --- a/src/mir/builder/control_flow/joinir/merge/mod.rs +++ b/src/mir/builder/control_flow/joinir/merge/mod.rs @@ -14,7 +14,6 @@ mod block_allocator; mod carrier_init_builder; -#[cfg(debug_assertions)] mod contract_checks; pub mod exit_line; mod exit_phi_builder; @@ -738,6 +737,48 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( debug, )?; + // Phase 118 P2: Contract check (Fail-Fast) - exit_bindings LoopState carriers must have exit PHIs. + if let Some(boundary) = boundary { + contract_checks::verify_exit_bindings_have_exit_phis(boundary, &exit_carrier_phis)?; + } + + // Phase 118 P1: Dev-only carrier-phi SSOT logs (exit_bindings vs carrier_inputs vs exit_carrier_phis) + if crate::config::env::joinir_dev_enabled() { + if let Some(boundary) = boundary { + let exit_binding_names: Vec<&str> = boundary + .exit_bindings + .iter() + .map(|b| b.carrier_name.as_str()) + .collect(); + let carrier_input_names: Vec<&str> = + merge_result.carrier_inputs.keys().map(|s| s.as_str()).collect(); + let exit_phi_names: Vec<&str> = + exit_carrier_phis.keys().map(|s| s.as_str()).collect(); + + trace.stderr_if( + &format!( + "[joinir/phase118/dev] exit_bindings carriers={:?}", + exit_binding_names + ), + true, + ); + trace.stderr_if( + &format!( + "[joinir/phase118/dev] carrier_inputs keys={:?}", + carrier_input_names + ), + true, + ); + trace.stderr_if( + &format!( + "[joinir/phase118/dev] exit_carrier_phis keys={:?}", + exit_phi_names + ), + true, + ); + } + } + // Phase 246-EX: CRITICAL FIX - Use exit PHI dsts for variable_map reconnection // // **Why EXIT PHI, not HEADER PHI?** diff --git a/src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs b/src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs index 56857927..be16cad2 100644 --- a/src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs +++ b/src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs @@ -127,13 +127,11 @@ pub fn lower_if_sum_pattern( // main() locals let i_init_val = alloc_value(); // i = 0 let sum_init_val = alloc_value(); // sum = 0 - let count_init_val = alloc_value(); // count = 0 (optional) let loop_result = alloc_value(); // result from loop_step // loop_step params let i_param = alloc_value(); let sum_param = alloc_value(); - let count_param = alloc_value(); // loop_step locals // Phase 220-D: loop_limit_val and if_value_val are already allocated by extract_*_condition() @@ -142,19 +140,14 @@ pub fn lower_if_sum_pattern( let exit_cond = alloc_value(); // negated loop condition let if_cmp = alloc_value(); // if condition comparison let sum_then = alloc_value(); // sum + update_addend - let count_const = alloc_value(); // count increment (1) - let count_then = alloc_value(); // count + 1 let const_0 = alloc_value(); // 0 for else branch let sum_else = alloc_value(); // sum + 0 (identity) - let count_else = alloc_value(); // count + 0 (identity) let sum_new = alloc_value(); // Select result for sum - let count_new = alloc_value(); // Select result for count let step_const = alloc_value(); // counter step let i_next = alloc_value(); // i + step // k_exit params let sum_final = alloc_value(); - let count_final = alloc_value(); // === main() function === let mut main_func = JoinFunction::new(main_id, "main".to_string(), vec![]); @@ -171,16 +164,10 @@ pub fn lower_if_sum_pattern( value: ConstValue::Integer(0), })); - // count_init = 0 - main_func.body.push(JoinInst::Compute(MirLikeInst::Const { - dst: count_init_val, - value: ConstValue::Integer(0), - })); - - // result = loop_step(i_init, sum_init, count_init) + // result = loop_step(i_init, sum_init) main_func.body.push(JoinInst::Call { func: loop_step_id, - args: vec![i_init_val, sum_init_val, count_init_val], + args: vec![i_init_val, sum_init_val], k_next: None, dst: Some(loop_result), }); @@ -191,11 +178,11 @@ pub fn lower_if_sum_pattern( join_module.add_function(main_func); - // === loop_step(i, sum, count) function === + // === loop_step(i, sum) function === let mut loop_step_func = JoinFunction::new( loop_step_id, "loop_step".to_string(), - vec![i_param, sum_param, count_param], + vec![i_param, sum_param], ); // --- Exit Condition Check --- @@ -230,7 +217,7 @@ pub fn lower_if_sum_pattern( // Jump to exit if condition is false loop_step_func.body.push(JoinInst::Jump { cont: k_exit_id.as_cont(), - args: vec![sum_param, count_param], + args: vec![sum_param], cond: Some(exit_cond), }); @@ -270,22 +257,6 @@ pub fn lower_if_sum_pattern( rhs: const_0, })); - // count_then = count + 1 - loop_step_func - .body - .push(JoinInst::Compute(MirLikeInst::Const { - dst: count_const, - value: ConstValue::Integer(1), - })); - loop_step_func - .body - .push(JoinInst::Compute(MirLikeInst::BinOp { - dst: count_then, - op: BinOpKind::Add, - lhs: count_param, - rhs: count_const, - })); - // --- Else Branch --- // sum_else = sum + 0 (identity) loop_step_func @@ -303,16 +274,6 @@ pub fn lower_if_sum_pattern( rhs: step_const, })); - // count_else = count + 0 (identity) - loop_step_func - .body - .push(JoinInst::Compute(MirLikeInst::BinOp { - dst: count_else, - op: BinOpKind::Add, - lhs: count_param, - rhs: step_const, // 0 - })); - // --- Select --- loop_step_func .body @@ -323,15 +284,6 @@ pub fn lower_if_sum_pattern( else_val: sum_else, })); - loop_step_func - .body - .push(JoinInst::Compute(MirLikeInst::Select { - dst: count_new, - cond: if_cmp, - then_val: count_then, - else_val: count_else, - })); - // --- Counter Update --- let step_const2 = alloc_value(); loop_step_func @@ -352,19 +304,15 @@ pub fn lower_if_sum_pattern( // --- Tail Recursion --- loop_step_func.body.push(JoinInst::Call { func: loop_step_id, - args: vec![i_next, sum_new, count_new], + args: vec![i_next, sum_new], k_next: None, dst: None, }); join_module.add_function(loop_step_func); - // === k_exit(sum_final, count_final) function === - let mut k_exit_func = JoinFunction::new( - k_exit_id, - "k_exit".to_string(), - vec![sum_final, count_final], - ); + // === k_exit(sum_final) function === + let mut k_exit_func = JoinFunction::new(k_exit_id, "k_exit".to_string(), vec![sum_final]); k_exit_func.body.push(JoinInst::Ret { value: Some(sum_final), @@ -374,9 +322,9 @@ pub fn lower_if_sum_pattern( join_module.entry = Some(main_id); // Build ExitMeta + // SSOT: use the accumulator name extracted from AST (no hardcoded carrier names). let mut exit_values = vec![]; - exit_values.push(("sum".to_string(), sum_final)); - exit_values.push(("count".to_string(), count_final)); + exit_values.push((update_var.clone(), sum_final)); let exit_meta = ExitMeta::multiple(exit_values);