fix(mir/exit_phi): Pass branch_source_block to build_exit_phis()

## Problem
Exit PHI generation was using header_id as predecessor, but when
build_expression(condition) creates new blocks, the actual branch
instruction is emitted from a different block, causing:
"phi pred mismatch: no input for predecessor BasicBlockId(X)"

## Solution
- Modified build_exit_phis() to accept branch_source_block parameter
- Capture actual block after condition evaluation in loop_builder.rs
- Use branch_source_block instead of header_id for PHI inputs

## Progress
- Error changed from ValueId(5941)/BasicBlockId(4674) to
  ValueId(5927)/BasicBlockId(4672), showing partial fix
- Added comprehensive test suite in mir_loopform_exit_phi.rs
- Added debug logging to trace condition block creation

## Status
Partial fix - unit tests pass, but Test 2 (Stage-B compilation) still
has errors. Needs further investigation of complex nested compilation
scenarios.

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

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
nyash-codex
2025-11-18 04:26:50 +09:00
parent 5bb094d58f
commit f92779cfe8
3 changed files with 251 additions and 6 deletions

View File

@ -105,6 +105,8 @@ impl<'a> LoopBuilder<'a> {
// Snapshot variables at break point for exit PHI generation // Snapshot variables at break point for exit PHI generation
let snapshot = self.get_current_variable_map(); let snapshot = self.get_current_variable_map();
let cur_block = self.current_block()?; let cur_block = self.current_block()?;
eprintln!("[DEBUG/do_break] Saved snapshot from block {:?}, vars: {:?}",
cur_block, snapshot.keys().collect::<Vec<_>>());
self.exit_snapshots.push((cur_block, snapshot)); self.exit_snapshots.push((cur_block, snapshot));
if let Some(exit_bb) = crate::mir::builder::loops::current_exit(self.parent_builder) { if let Some(exit_bb) = crate::mir::builder::loops::current_exit(self.parent_builder) {
@ -237,8 +239,20 @@ impl<'a> LoopBuilder<'a> {
self.exit_snapshots.clear(); self.exit_snapshots.clear();
// Emit condition check in header // Emit condition check in header
if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() {
eprintln!("[loopform/condition] BEFORE build_expression: current_block={:?}", self.current_block()?);
}
let cond_value = self.parent_builder.build_expression(condition)?; let cond_value = self.parent_builder.build_expression(condition)?;
// Capture the ACTUAL block that emits the branch (might differ from header_id
// if build_expression created new blocks)
let branch_source_block = self.current_block()?;
if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() {
eprintln!("[loopform/condition] AFTER build_expression: branch_source_block={:?}", branch_source_block);
}
self.emit_branch(cond_value, body_id, exit_id)?; self.emit_branch(cond_value, body_id, exit_id)?;
if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() {
eprintln!("[loopform/condition] AFTER emit_branch: current_block={:?}", self.current_block()?);
}
// Lower loop body // Lower loop body
self.set_current_block(body_id)?; self.set_current_block(body_id)?;
@ -280,7 +294,7 @@ impl<'a> LoopBuilder<'a> {
// Build exit PHIs for break statements // Build exit PHIs for break statements
let exit_snaps = self.exit_snapshots.clone(); let exit_snaps = self.exit_snapshots.clone();
loopform.build_exit_phis(self, exit_id, &exit_snaps)?; loopform.build_exit_phis(self, exit_id, branch_source_block, &exit_snaps)?;
// Pop loop context // Pop loop context
crate::mir::builder::loops::pop_loop_context(self.parent_builder); crate::mir::builder::loops::pop_loop_context(self.parent_builder);
@ -341,7 +355,7 @@ impl<'a> LoopBuilder<'a> {
// 4. ループ変数のPhi nodeを準備 // 4. ループ変数のPhi nodeを準備
// ここでは、ループ内で変更される可能性のある変数を事前に検出するか、 // ここでは、ループ内で変更される可能性のある変数を事前に検出するか、
// または変数アクセス時に遅延生成する(再束縛は条件式構築後に行う) // または変数アクセス時に遅延生成する(再束縛は条件式構築後に行う)
let incs = self.prepare_loop_variables(header_id, preheader_id, &pre_vars_snapshot)?; let incs = self.prepare_loop_variables(header_id, preheader_id, &pre_vars_snapshot, &assigned_vars)?;
// 5. 条件評価Phi nodeの結果を使用 // 5. 条件評価Phi nodeの結果を使用
// Heuristic pre-pin: if condition is a comparison, evaluate its operands and pin them // Heuristic pre-pin: if condition is a comparison, evaluate its operands and pin them
@ -566,18 +580,36 @@ impl<'a> LoopBuilder<'a> {
// PHI Helpers — prepare/finalize PHIs and block sealing // PHI Helpers — prepare/finalize PHIs and block sealing
// ============================================================= // =============================================================
/// ループ変数の準備(事前検出または遅延生成) /// ループ変数の準備(事前検出または遅延生成)
///
/// ポリシー:
/// - ループキャリア(ループ本体で再代入される変数)と pinned 変数のみを PHI 対象とする。
/// - ループ不変のローカルtext_len / pattern_len など)は preheader 値をそのまま使い、
/// 不要な PHI を張らないことで SSA 破綻(同一 ValueId の二重定義)を防ぐ。
fn prepare_loop_variables( fn prepare_loop_variables(
&mut self, &mut self,
header_id: BasicBlockId, header_id: BasicBlockId,
preheader_id: BasicBlockId, preheader_id: BasicBlockId,
pre_vars_snapshot: &std::collections::HashMap<String, ValueId>, pre_vars_snapshot: &std::collections::HashMap<String, ValueId>,
assigned_vars: &[String],
) -> Result<Vec<crate::mir::phi_core::loop_phi::IncompletePhi>, String> { ) -> Result<Vec<crate::mir::phi_core::loop_phi::IncompletePhi>, String> {
use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::atomic::{AtomicUsize, Ordering};
static CALL_COUNT: AtomicUsize = AtomicUsize::new(0); static CALL_COUNT: AtomicUsize = AtomicUsize::new(0);
let count = CALL_COUNT.fetch_add(1, Ordering::SeqCst); let count = CALL_COUNT.fetch_add(1, Ordering::SeqCst);
// Use the variable map captured at preheader (before switching to header) // Use the variable map captured at preheader (before switching to header),
let current_vars = pre_vars_snapshot.clone(); // but filter to:
// - ループキャリアassigned_vars に含まれる変数)
// - pinned 変数__pin$*: 受信箱など、ループをまたいで値を運ぶ必要があるもの
let mut current_vars = std::collections::HashMap::new();
for (name, &val) in pre_vars_snapshot.iter() {
if name.starts_with("__pin$") {
current_vars.insert(name.clone(), val);
continue;
}
if assigned_vars.iter().any(|v| v == name) {
current_vars.insert(name.clone(), val);
}
}
// Debug: print current_vars before prepare (guarded by env) // Debug: print current_vars before prepare (guarded by env)
let dbg = std::env::var("NYASH_BUILDER_DEBUG").ok().as_deref() == Some("1"); let dbg = std::env::var("NYASH_BUILDER_DEBUG").ok().as_deref() == Some("1");
if dbg { if dbg {

View File

@ -222,29 +222,50 @@ impl LoopFormBuilder {
/// Similar to header PHIs, but merges: /// Similar to header PHIs, but merges:
/// - Header fallthrough (normal loop exit) /// - Header fallthrough (normal loop exit)
/// - Break snapshots (early exit from loop body) /// - Break snapshots (early exit from loop body)
///
/// # Parameters
/// - `branch_source_block`: The ACTUAL block that emitted the branch to exit
/// (might differ from header_id if condition evaluation created new blocks)
pub fn build_exit_phis<O: LoopFormOps>( pub fn build_exit_phis<O: LoopFormOps>(
&self, &self,
ops: &mut O, ops: &mut O,
exit_id: BasicBlockId, exit_id: BasicBlockId,
branch_source_block: BasicBlockId,
exit_snapshots: &[(BasicBlockId, HashMap<String, ValueId>)], exit_snapshots: &[(BasicBlockId, HashMap<String, ValueId>)],
) -> Result<(), String> { ) -> Result<(), String> {
ops.set_current_block(exit_id)?; ops.set_current_block(exit_id)?;
let debug = std::env::var("NYASH_LOOPFORM_DEBUG").is_ok();
if debug {
eprintln!("[DEBUG/exit_phi] ====== Exit PHI Generation ======");
eprintln!("[DEBUG/exit_phi] exit_id = {:?}, header_id = {:?}, branch_source = {:?}",
exit_id, self.header_id, branch_source_block);
eprintln!("[DEBUG/exit_phi] exit_snapshots.len() = {}", exit_snapshots.len());
for (i, (bb, snap)) in exit_snapshots.iter().enumerate() {
eprintln!("[DEBUG/exit_phi] snapshot[{}]: block = {:?}, num_vars = {}",
i, bb, snap.len());
}
eprintln!("[DEBUG/exit_phi] pinned.len() = {}, carriers.len() = {}",
self.pinned.len(), self.carriers.len());
}
// Collect all variables that need exit PHIs // Collect all variables that need exit PHIs
let mut all_vars: HashMap<String, Vec<(BasicBlockId, ValueId)>> = HashMap::new(); let mut all_vars: HashMap<String, Vec<(BasicBlockId, ValueId)>> = HashMap::new();
// Add header fallthrough values (pinned + carriers) // Add header fallthrough values (pinned + carriers)
// Use branch_source_block instead of header_id because condition evaluation
// might create new blocks, and the branch is emitted from the LAST block
for pinned in &self.pinned { for pinned in &self.pinned {
all_vars all_vars
.entry(pinned.name.clone()) .entry(pinned.name.clone())
.or_default() .or_default()
.push((self.header_id, pinned.header_phi)); .push((branch_source_block, pinned.header_phi));
} }
for carrier in &self.carriers { for carrier in &self.carriers {
all_vars all_vars
.entry(carrier.name.clone()) .entry(carrier.name.clone())
.or_default() .or_default()
.push((self.header_id, carrier.header_phi)); .push((branch_source_block, carrier.header_phi));
} }
// Add break snapshot values // Add break snapshot values
@ -262,15 +283,33 @@ impl LoopFormBuilder {
// Deduplicate inputs by predecessor block // Deduplicate inputs by predecessor block
sanitize_phi_inputs(&mut inputs); sanitize_phi_inputs(&mut inputs);
if debug {
eprintln!("[DEBUG/exit_phi] Variable '{}': {} inputs before dedup", var_name, inputs.len());
for (bb, val) in &inputs {
eprintln!("[DEBUG/exit_phi] pred={:?} val={:?}", bb, val);
}
}
match inputs.len() { match inputs.len() {
0 => {} // No inputs, skip 0 => {} // No inputs, skip
1 => { 1 => {
// Single predecessor: direct binding // Single predecessor: direct binding
if debug {
eprintln!("[DEBUG/exit_phi] Variable '{}': single predecessor, direct binding to {:?}",
var_name, inputs[0].1);
}
ops.update_var(var_name, inputs[0].1); ops.update_var(var_name, inputs[0].1);
} }
_ => { _ => {
// Multiple predecessors: create PHI node // Multiple predecessors: create PHI node
let phi_id = ops.new_value(); let phi_id = ops.new_value();
if debug {
eprintln!("[DEBUG/exit_phi] Creating PHI {:?} for var '{}' with {} inputs",
phi_id, var_name, inputs.len());
for (bb, val) in &inputs {
eprintln!("[DEBUG/exit_phi] PHI input: pred={:?} val={:?}", bb, val);
}
}
ops.emit_phi(phi_id, inputs)?; ops.emit_phi(phi_id, inputs)?;
ops.update_var(var_name, phi_id); ops.update_var(var_name, phi_id);
} }

View File

@ -0,0 +1,174 @@
/*!
* Unit tests for LoopForm v2 exit PHI generation
*
* Tests the build_exit_phis() implementation in loopform_builder.rs
* Focus: predecessor tracking and PHI input generation for break statements
*/
use crate::parser::NyashParser;
use crate::mir::{MirCompiler, MirVerifier};
#[test]
fn test_loopform_exit_phi_single_break() {
// Enable LoopForm PHI v2 and MIR verification
std::env::set_var("NYASH_LOOPFORM_PHI_V2", "1");
std::env::set_var("NYASH_VM_VERIFY_MIR", "1");
std::env::set_var("NYASH_LOOPFORM_DEBUG", "1");
std::env::set_var("NYASH_PARSER_STAGE3", "1");
std::env::set_var("NYASH_PARSER_ALLOW_SEMICOLON", "1");
let src = r#"
static box TestExitPhi {
test() {
local i = 0
loop(i < 10) {
if i == 5 { break }
i = i + 1
}
return i
}
}
"#;
println!("=== Test: Single break statement ===");
// Parse
let ast = NyashParser::parse_from_string(src)
.expect("parse failed");
// Compile
let mut mc = MirCompiler::with_options(false);
let cr = mc.compile(ast).expect("compile failed");
// MIR verification
let mut verifier = MirVerifier::new();
if let Err(errors) = verifier.verify_module(&cr.module) {
for err in &errors {
eprintln!("❌ MIR verification error: {}", err);
}
panic!("❌ MIR verification failed with {} errors", errors.len());
}
println!("✅ MIR verification passed");
}
#[test]
fn test_loopform_exit_phi_multiple_breaks() {
std::env::set_var("NYASH_LOOPFORM_PHI_V2", "1");
std::env::set_var("NYASH_VM_VERIFY_MIR", "1");
std::env::set_var("NYASH_LOOPFORM_DEBUG", "1");
std::env::set_var("NYASH_PARSER_STAGE3", "1");
std::env::set_var("NYASH_PARSER_ALLOW_SEMICOLON", "1");
let src = r#"
static box TestMultiBreak {
test() {
local i = 0
loop(i < 10) {
if i == 3 { break }
if i == 5 { break }
i = i + 1
}
return i
}
}
"#;
println!("=== Test: Multiple break statements ===");
let ast = NyashParser::parse_from_string(src).expect("parse failed");
let mut mc = MirCompiler::with_options(false);
let cr = mc.compile(ast).expect("compile failed");
let mut verifier = MirVerifier::new();
if let Err(errors) = verifier.verify_module(&cr.module) {
for err in &errors {
eprintln!("❌ MIR verification error: {}", err);
}
panic!("❌ MIR verification failed with {} errors", errors.len());
}
println!("✅ MIR verification passed");
}
#[test]
fn test_loopform_exit_phi_nested_if_break() {
std::env::set_var("NYASH_LOOPFORM_PHI_V2", "1");
std::env::set_var("NYASH_VM_VERIFY_MIR", "1");
std::env::set_var("NYASH_LOOPFORM_DEBUG", "1");
std::env::set_var("NYASH_PARSER_STAGE3", "1");
std::env::set_var("NYASH_PARSER_ALLOW_SEMICOLON", "1");
let src = r#"
static box TestNestedBreak {
test() {
local i = 0
local found = 0
loop(i < 10) {
if i > 5 {
if i == 7 {
found = 1
break
}
}
i = i + 1
}
return found
}
}
"#;
println!("=== Test: Nested if with break ===");
let ast = NyashParser::parse_from_string(src).expect("parse failed");
let mut mc = MirCompiler::with_options(false);
let cr = mc.compile(ast).expect("compile failed");
let mut verifier = MirVerifier::new();
if let Err(errors) = verifier.verify_module(&cr.module) {
for err in &errors {
eprintln!("❌ MIR verification error: {}", err);
}
panic!("❌ MIR verification failed with {} errors", errors.len());
}
println!("✅ MIR verification passed");
}
#[test]
fn test_loopform_exit_phi_multiple_vars() {
std::env::set_var("NYASH_LOOPFORM_PHI_V2", "1");
std::env::set_var("NYASH_VM_VERIFY_MIR", "1");
std::env::set_var("NYASH_LOOPFORM_DEBUG", "1");
std::env::set_var("NYASH_PARSER_STAGE3", "1");
std::env::set_var("NYASH_PARSER_ALLOW_SEMICOLON", "1");
let src = r#"
static box TestMultiVars {
test() {
local i = 0
local sum = 0
local product = 1
loop(i < 10) {
if sum > 20 { break }
sum = sum + i
product = product * 2
i = i + 1
}
return sum
}
}
"#;
println!("=== Test: Multiple variables with break ===");
let ast = NyashParser::parse_from_string(src).expect("parse failed");
let mut mc = MirCompiler::with_options(false);
let cr = mc.compile(ast).expect("compile failed");
let mut verifier = MirVerifier::new();
if let Err(errors) = verifier.verify_module(&cr.module) {
for err in &errors {
eprintln!("❌ MIR verification error: {}", err);
}
panic!("❌ MIR verification failed with {} errors", errors.len());
}
println!("✅ MIR verification passed");
}