feat(joinir): Phase 132-P3 - Exit PHI collision early detection

Added verify_exit_phi_no_collision() to contract_checks.rs for early detection
of ValueId collisions between exit PHIs and other instructions.

Problem detected:
- If exit_phi_builder uses builder.value_gen.next() (module-level) instead of
  func.next_value_id() (function-level), ValueIds can collide:

  Example:
  - bb0: %1 = const 0   (counter init)
  - bb3: %1 = phi ...   (exit PHI - collision!)

Previous behavior:
- Error only detected at LLVM backend runtime
- Cryptic error: "Cannot overwrite PHI dst=1"

New behavior:
- Panic at Rust compile time (debug build)
- Clear error message with fix suggestion:
  "Exit PHI dst %1 collides with instruction in block 0
   Fix: Use func.next_value_id() in exit_phi_builder.rs"

Benefits:
- 🔥 Fail-Fast: Catch errors during Rust compilation, not LLVM execution
- 📋 Clear messages: Exact collision point + fix suggestion
- 🧪 Testable: verify_exit_phi_no_collision() can be unit tested

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
nyash-codex
2025-12-15 06:00:48 +09:00
parent bd07b7f41f
commit 771bf6b0d1
7 changed files with 211 additions and 16 deletions

View File

@ -96,6 +96,131 @@ pub(super) fn verify_exit_line(
}
}
}
// Phase 132-P2: Verify exit PHI ValueIds don't collide with other instructions
verify_exit_phi_no_collision(func, exit_block);
}
/// Phase 132-P2: Verify exit PHI dst ValueIds don't collide with other instructions
///
/// # Problem
///
/// If exit_phi_builder uses builder.value_gen.next() (module-level) instead of
/// func.next_value_id() (function-level), it can allocate ValueIds that collide
/// with existing instructions in the function.
///
/// Example collision:
/// - bb0: %1 = const 0 (counter init)
/// - bb3: %1 = phi ... (exit PHI - collision!)
///
/// This causes LLVM backend errors:
/// "Cannot overwrite PHI dst=1. ValueId namespace collision detected."
///
/// # Contract
///
/// All exit PHI dst ValueIds must be unique within the function and not
/// overwrite any existing instruction dst.
///
/// # Panics
///
/// Panics if any exit PHI dst collides with an existing instruction dst.
#[cfg(debug_assertions)]
fn verify_exit_phi_no_collision(func: &MirFunction, exit_block: BasicBlockId) {
let exit_block_data = match func.blocks.get(&exit_block) {
Some(block) => block,
None => return, // Block not found, other verification will catch this
};
// Collect all exit PHI dsts
let mut exit_phi_dsts = std::collections::HashSet::new();
for instr in &exit_block_data.instructions {
if let MirInstruction::Phi { dst, .. } = instr {
exit_phi_dsts.insert(*dst);
}
}
if exit_phi_dsts.is_empty() {
return; // No exit PHIs, nothing to verify
}
// Collect all instruction dsts in the entire function (excluding PHIs)
let mut all_non_phi_dsts = std::collections::HashSet::new();
for (block_id, block) in &func.blocks {
if *block_id == exit_block {
// For exit block, only check non-PHI instructions
for instr in &block.instructions {
if !matches!(instr, MirInstruction::Phi { .. }) {
if let Some(dst) = get_instruction_dst(instr) {
all_non_phi_dsts.insert(dst);
}
}
}
} else {
// For other blocks, check all instructions
for instr in &block.instructions {
if let Some(dst) = get_instruction_dst(instr) {
all_non_phi_dsts.insert(dst);
}
}
}
}
// Check for collisions
for phi_dst in &exit_phi_dsts {
if all_non_phi_dsts.contains(phi_dst) {
// Find which instruction collides
for (block_id, block) in &func.blocks {
for instr in &block.instructions {
if matches!(instr, MirInstruction::Phi { .. }) && *block_id == exit_block {
continue; // Skip exit PHIs themselves
}
if let Some(dst) = get_instruction_dst(instr) {
if dst == *phi_dst {
panic!(
"[JoinIRVerifier/Phase132-P2] Exit PHI dst {:?} collides with instruction in block {}: {:?}\n\
This indicates exit_phi_builder used module-level value_gen.next() instead of function-level next_value_id().\n\
Fix: Use func.next_value_id() in exit_phi_builder.rs",
phi_dst, block_id.0, instr
);
}
}
}
}
}
}
}
/// Helper: Extract dst ValueId from MirInstruction
#[cfg(debug_assertions)]
fn get_instruction_dst(instr: &MirInstruction) -> Option<ValueId> {
use MirInstruction;
match instr {
MirInstruction::Const { dst, .. }
| MirInstruction::Load { dst, .. }
| MirInstruction::UnaryOp { dst, .. }
| MirInstruction::BinOp { dst, .. }
| MirInstruction::Compare { dst, .. }
| MirInstruction::TypeOp { dst, .. }
| MirInstruction::NewBox { dst, .. }
| MirInstruction::NewClosure { dst, .. }
| MirInstruction::Copy { dst, .. }
| MirInstruction::Cast { dst, .. }
| MirInstruction::TypeCheck { dst, .. }
| MirInstruction::Phi { dst, .. }
| MirInstruction::ArrayGet { dst, .. }
| MirInstruction::RefNew { dst, .. }
| MirInstruction::RefGet { dst, .. }
| MirInstruction::WeakNew { dst, .. }
| MirInstruction::WeakLoad { dst, .. }
| MirInstruction::WeakRef { dst, .. }
| MirInstruction::FutureNew { dst, .. }
| MirInstruction::Await { dst, .. } => Some(*dst),
MirInstruction::BoxCall { dst, .. }
| MirInstruction::ExternCall { dst, .. }
| MirInstruction::Call { dst, .. }
| MirInstruction::PluginInvoke { dst, .. } => *dst,
_ => None,
}
}
#[cfg(debug_assertions)]