refactor(joinir): Phase 189 - Remove hardcoded 'sum' variable, generalize exit PHI connection
Key improvements: 1. **Eliminate hardcoded variable name**: Replace hardcoded "sum" with generic ValueId-based variable_map updates. Add new JoinInlineBoundary constructor `new_with_input_and_host_outputs()` for Pattern 3. 2. **Generalize output slot mapping**: Exit PHI result now updates all host_outputs entries in variable_map, not just hardcoded "sum". Prepares for future multi-carrier patterns. 3. **PHI preservation in blocks**: Fix block finalization to preserve existing PHI instructions (from handle_select) instead of overwriting them. 4. **Stable function name→ValueId mapping**: Ensure consistent ValueId assignment for tail call detection across multi-function merges. 5. **Enhanced debugging**: Add detailed logging in block converter and meta analysis for PHI verification. Files modified: - src/mir/builder/control_flow.rs: Remove hardcoded "sum", use boundary outputs - src/mir/join_ir/lowering/inline_boundary.rs: Add new constructor - src/mir/join_ir_vm_bridge/joinir_block_converter.rs: Stable mappings, PHI preservation - src/mir/join_ir_vm_bridge/meta.rs: Debug output for PHI tracking - src/mir/builder/joinir_id_remapper.rs: PHI value remapping - src/mir/builder/joinir_inline_boundary_injector.rs: Span preservation Test status: Pattern 3 (loop_if_phi.hako) still produces correct result (sum=9, RC=9) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: ChatGPT <noreply@openai.com> Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@ -795,29 +795,17 @@ impl super::MirBuilder {
|
||||
// Phase 188-Impl-3: Create and pass JoinInlineBoundary for Pattern 3
|
||||
// Pattern 3 has TWO carriers: i and sum
|
||||
self.trace_varmap("pattern3_before_merge");
|
||||
let boundary = crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary::new_inputs_only(
|
||||
let boundary = crate::mir::join_ir::lowering::inline_boundary::JoinInlineBoundary::new_with_input_and_host_outputs(
|
||||
vec![ValueId(0), ValueId(1)], // JoinIR's main() parameters (i, sum init)
|
||||
vec![loop_var_id, sum_var_id], // Host's loop variables
|
||||
vec![sum_var_id], // Host output slot to be updated with exit PHI
|
||||
);
|
||||
let exit_phi_result = self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?;
|
||||
self.trace_varmap("pattern3_after_merge");
|
||||
|
||||
// Phase 189-Fix: Update variable_map["sum"] to point to exit PHI result
|
||||
// The exit PHI contains the final value of sum after the loop completes.
|
||||
// This ensures subsequent references to "sum" (e.g., `return sum`) use the correct value.
|
||||
if let Some(exit_phi) = exit_phi_result {
|
||||
self.variable_map.insert("sum".to_string(), exit_phi);
|
||||
if debug {
|
||||
eprintln!(
|
||||
"[cf_loop/joinir/pattern3] Updated variable_map['sum'] to exit PHI {:?}",
|
||||
exit_phi
|
||||
);
|
||||
}
|
||||
self.trace_varmap("pattern3_exit_phi_connected");
|
||||
}
|
||||
|
||||
// Phase 188-Impl-3: Return Void (the loop itself doesn't produce a value, but its result
|
||||
// is now accessible via variable_map["sum"])
|
||||
// Phase 189-Refine: variable_map の更新は merge_joinir_mir_blocks 内で
|
||||
// JoinInlineBoundary.host_outputs を用いて行われる。
|
||||
// この関数では Void を返すだけでよい(戻り値は後続の `return sum` が扱う)。
|
||||
let void_val = crate::mir::builder::emission::constant::emit_void(self);
|
||||
|
||||
if debug {
|
||||
@ -864,8 +852,9 @@ impl super::MirBuilder {
|
||||
/// # Returns
|
||||
///
|
||||
/// Returns `Ok(Some(exit_phi_id))` if the merged JoinIR functions have return values
|
||||
/// that were collected into an exit block PHI. This allows the caller to connect
|
||||
/// the exit PHI result to the appropriate host variable.
|
||||
/// that were collected into an exit block PHI. さらに、`boundary` に
|
||||
/// host_outputs が指定されている場合は、exit PHI の結果をホスト側の
|
||||
/// SSA スロットへ再接続する(variable_map 内の ValueId を更新する)。
|
||||
fn merge_joinir_mir_blocks(
|
||||
&mut self,
|
||||
mir_module: &crate::mir::MirModule,
|
||||
@ -1349,19 +1338,29 @@ impl super::MirBuilder {
|
||||
};
|
||||
|
||||
// Phase 189-Fix: Store exit PHI result in variable_map so host code can reference it
|
||||
// The loop result should update the corresponding carrier variable
|
||||
// The loop result should update the corresponding carrier variable(s) declared
|
||||
// in JoinInlineBoundary.host_outputs.
|
||||
if let Some(phi_result) = exit_phi_result_id {
|
||||
// Use boundary output info to connect exit PHI to host variables
|
||||
if let Some(ref boundary) = boundary {
|
||||
// If boundary has outputs, map JoinIR output to host output
|
||||
if !boundary.join_outputs.is_empty() && !boundary.host_outputs.is_empty() {
|
||||
// For now, we assume single output (sum in the pattern)
|
||||
// Future: support multiple outputs
|
||||
if debug {
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] Would connect exit PHI {:?} to host output {:?}",
|
||||
phi_result, boundary.host_outputs
|
||||
);
|
||||
// Phase 189-Refine: 現時点では単一出力 (Pattern 3 の sum) のみを想定し、
|
||||
// host_outputs に登録された ValueId と同じ値を持つ variable_map の
|
||||
// エントリを exit PHI 結果に差し替える。
|
||||
//
|
||||
// 将来 Multi-carrier を扱う際は、join_outputs と host_outputs の
|
||||
// ペアに対して同様の処理を行う。
|
||||
for &host_out in &boundary.host_outputs {
|
||||
// variable_map は name → ValueId のマップなので、
|
||||
// 値が host_out になっているものを exit PHI に更新する。
|
||||
for (name, vid) in self.variable_map.iter_mut() {
|
||||
if *vid == host_out {
|
||||
*vid = phi_result;
|
||||
if debug {
|
||||
eprintln!(
|
||||
"[cf_loop/joinir] Updated variable_map['{}'] to exit PHI {:?}",
|
||||
name, phi_result
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -323,8 +323,17 @@ impl JoinIrIdRemapper {
|
||||
args: args.iter().map(|&a| remap(a)).collect(),
|
||||
effects: *effects,
|
||||
},
|
||||
// Pass through unchanged
|
||||
Branch { .. } | Jump { .. } | Return { .. } | Phi { .. } | Nop | Safepoint => inst.clone(),
|
||||
// Phase 189 FIX: Remap PHI dst and input values (BlockId remapping is done in control_flow.rs)
|
||||
Phi { dst, inputs, type_hint } => Phi {
|
||||
dst: remap(*dst),
|
||||
inputs: inputs
|
||||
.iter()
|
||||
.map(|(bb, val)| (*bb, remap(*val)))
|
||||
.collect(),
|
||||
type_hint: type_hint.clone(),
|
||||
},
|
||||
// Pass through unchanged (Branch/Jump/Return handled separately)
|
||||
Branch { .. } | Jump { .. } | Return { .. } | Nop | Safepoint => inst.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -81,8 +81,13 @@ impl BoundaryInjector {
|
||||
|
||||
// Entry block の先頭に Copy instructions を挿入
|
||||
// Reverse order to preserve original order when inserting at position 0
|
||||
// Phase 189 FIX: Also insert corresponding spans
|
||||
let default_span = entry_block.instruction_spans.first()
|
||||
.copied()
|
||||
.unwrap_or_else(crate::ast::Span::unknown);
|
||||
for inst in copy_instructions.into_iter().rev() {
|
||||
entry_block.instructions.insert(0, inst);
|
||||
entry_block.instruction_spans.insert(0, default_span);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
|
||||
@ -79,15 +79,20 @@ pub struct JoinInlineBoundary {
|
||||
/// For loops that produce values (e.g., loop result), these are the
|
||||
/// JoinIR-local ValueIds that should be visible to the host after inlining.
|
||||
///
|
||||
/// Currently unused for Pattern 1 (Simple While), reserved for future patterns.
|
||||
/// Phase 188/189 ではまだ利用していないが、将来的な Multi-carrier パターン
|
||||
/// (複数の変数を一度に返すループ) のために予約している。
|
||||
pub join_outputs: Vec<ValueId>,
|
||||
|
||||
/// Host-function ValueIds that receive the outputs
|
||||
///
|
||||
/// These are the destination ValueIds in the host function that should
|
||||
/// receive the values from join_outputs.
|
||||
/// receive the values from join_outputs, or (Pattern 3 のような単一
|
||||
/// キャリアのケースでは) ループ exit PHI の結果を受け取るホスト側の
|
||||
/// SSA スロットを表す。
|
||||
///
|
||||
/// Currently unused for Pattern 1 (Simple While), reserved for future patterns.
|
||||
/// Phase 188-Impl-3 までは未使用だったが、Phase 189 で
|
||||
/// loop_if_phi.hako の sum のような「ループの出口で更新されるキャリア」の
|
||||
/// 再接続に利用する。
|
||||
pub host_outputs: Vec<ValueId>,
|
||||
}
|
||||
|
||||
@ -114,6 +119,9 @@ impl JoinInlineBoundary {
|
||||
/// Create a new boundary with both inputs and outputs
|
||||
///
|
||||
/// Reserved for future loop patterns that produce values.
|
||||
///
|
||||
/// 現在の実装では Multi-carrier 出力には未対応だが、型としては複数出力を
|
||||
/// 表現できるようにしておく。
|
||||
#[allow(dead_code)]
|
||||
pub fn new_with_outputs(
|
||||
join_inputs: Vec<ValueId>,
|
||||
@ -138,6 +146,33 @@ impl JoinInlineBoundary {
|
||||
host_outputs,
|
||||
}
|
||||
}
|
||||
|
||||
/// Create a new boundary with inputs and **host outputs only**
|
||||
///
|
||||
/// JoinIR 側の exit 値 (k_exit の引数など) を 1 つの PHI にまとめ、
|
||||
/// その PHI 結果をホスト側の変数スロットへ再接続したい場合に使う。
|
||||
///
|
||||
/// 典型例: Pattern 3 (loop_if_phi.hako)
|
||||
/// - join_inputs : [i_init, sum_init]
|
||||
/// - host_inputs : [host_i, host_sum]
|
||||
/// - host_outputs : [host_sum] // ループ exit 時に上書きしたい変数
|
||||
pub fn new_with_input_and_host_outputs(
|
||||
join_inputs: Vec<ValueId>,
|
||||
host_inputs: Vec<ValueId>,
|
||||
host_outputs: Vec<ValueId>,
|
||||
) -> Self {
|
||||
assert_eq!(
|
||||
join_inputs.len(),
|
||||
host_inputs.len(),
|
||||
"join_inputs and host_inputs must have same length"
|
||||
);
|
||||
Self {
|
||||
join_inputs,
|
||||
host_inputs,
|
||||
join_outputs: vec![],
|
||||
host_outputs,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@ -15,6 +15,10 @@ pub struct JoinIrBlockConverter {
|
||||
current_block_id: BasicBlockId,
|
||||
current_instructions: Vec<MirInstruction>,
|
||||
next_block_id: u32,
|
||||
// Phase 189: Stable function name → ValueId mapping for tail call detection
|
||||
// Ensures the same function name always gets the same ValueId
|
||||
func_name_to_value_id: std::collections::HashMap<String, ValueId>,
|
||||
next_func_name_value_id: u32,
|
||||
}
|
||||
|
||||
impl JoinIrBlockConverter {
|
||||
@ -23,6 +27,8 @@ impl JoinIrBlockConverter {
|
||||
current_block_id: BasicBlockId(0), // entry block
|
||||
current_instructions: Vec::new(),
|
||||
next_block_id: 1, // start from 1 (0 is entry)
|
||||
func_name_to_value_id: std::collections::HashMap::new(),
|
||||
next_func_name_value_id: 99990, // Use 99990+ range for function names
|
||||
}
|
||||
}
|
||||
|
||||
@ -42,6 +48,16 @@ impl JoinIrBlockConverter {
|
||||
for join_inst in join_body {
|
||||
match join_inst {
|
||||
JoinInst::Compute(mir_like) => {
|
||||
// Phase 189: Special handling for MirLikeInst::Select
|
||||
// Pattern 3 uses JoinInst::Compute(MirLikeInst::Select {...})
|
||||
// but Select needs control flow expansion (Branch + Phi), not single instruction
|
||||
if let MirLikeInst::Select { dst, cond, then_val, else_val } = mir_like {
|
||||
eprintln!("[joinir_block] ✅ Found Select! dst={:?}, calling handle_select", dst);
|
||||
self.handle_select(mir_func, dst, cond, then_val, else_val, &None)?;
|
||||
continue;
|
||||
}
|
||||
// Debug: show what instruction we're processing
|
||||
eprintln!("[joinir_block] Compute instruction: {:?}", mir_like);
|
||||
let mir_inst = convert_mir_like_inst(mir_like)?;
|
||||
self.current_instructions.push(mir_inst);
|
||||
}
|
||||
@ -272,8 +288,17 @@ impl JoinIrBlockConverter {
|
||||
}
|
||||
|
||||
let func_name = join_func_name(*func);
|
||||
let func_name_id = ValueId(99990 + self.next_block_id);
|
||||
self.next_block_id += 1;
|
||||
|
||||
// Phase 189: Use stable function name → ValueId mapping
|
||||
// This ensures the same function name always gets the same ValueId,
|
||||
// which is critical for tail call detection in merge_joinir_mir_blocks
|
||||
let func_name_id = *self.func_name_to_value_id
|
||||
.entry(func_name.clone())
|
||||
.or_insert_with(|| {
|
||||
let id = ValueId(self.next_func_name_value_id);
|
||||
self.next_func_name_value_id += 1;
|
||||
id
|
||||
});
|
||||
|
||||
self.current_instructions.push(MirInstruction::Const {
|
||||
dst: func_name_id,
|
||||
@ -437,8 +462,23 @@ impl JoinIrBlockConverter {
|
||||
type_hint: type_hint.clone(),
|
||||
});
|
||||
merge_block_obj.instruction_spans.push(Span::unknown());
|
||||
eprintln!(
|
||||
"[joinir_block/handle_select] Created merge_block {:?} with {} instructions (first={:?})",
|
||||
merge_block,
|
||||
merge_block_obj.instructions.len(),
|
||||
merge_block_obj.instructions.first()
|
||||
);
|
||||
mir_func.blocks.insert(merge_block, merge_block_obj);
|
||||
|
||||
// Verify PHI was inserted
|
||||
if let Some(inserted) = mir_func.blocks.get(&merge_block) {
|
||||
eprintln!(
|
||||
"[joinir_block/handle_select] After insert: merge_block {:?} has {} instructions",
|
||||
merge_block,
|
||||
inserted.instructions.len()
|
||||
);
|
||||
}
|
||||
|
||||
self.current_block_id = merge_block;
|
||||
Ok(())
|
||||
}
|
||||
@ -654,10 +694,34 @@ impl JoinIrBlockConverter {
|
||||
instructions: Vec<MirInstruction>,
|
||||
terminator: MirInstruction,
|
||||
) {
|
||||
eprintln!("[joinir_block/finalize_block] block_id={:?}, instructions.len()={}", block_id, instructions.len());
|
||||
if let Some(block) = mir_func.blocks.get_mut(&block_id) {
|
||||
let inst_count = instructions.len();
|
||||
block.instructions = instructions;
|
||||
block.instruction_spans = vec![Span::unknown(); inst_count];
|
||||
// Phase 189 FIX: Preserve existing PHI instructions at block start
|
||||
// PHI instructions must remain at the beginning of the block
|
||||
let existing_phis: Vec<_> = block
|
||||
.instructions
|
||||
.iter()
|
||||
.filter(|i| matches!(i, MirInstruction::Phi { .. }))
|
||||
.cloned()
|
||||
.collect();
|
||||
let phi_count = existing_phis.len();
|
||||
|
||||
if phi_count > 0 {
|
||||
eprintln!(
|
||||
"[joinir_block/finalize_block] Preserving {} PHI instructions in block {:?}",
|
||||
phi_count, block_id
|
||||
);
|
||||
// PHI first, then new instructions
|
||||
let mut merged = existing_phis;
|
||||
merged.extend(instructions);
|
||||
let total_count = merged.len();
|
||||
block.instructions = merged;
|
||||
block.instruction_spans = vec![Span::unknown(); total_count];
|
||||
} else {
|
||||
let inst_count = instructions.len();
|
||||
block.instructions = instructions;
|
||||
block.instruction_spans = vec![Span::unknown(); inst_count];
|
||||
}
|
||||
block.terminator = Some(terminator);
|
||||
}
|
||||
}
|
||||
@ -670,9 +734,12 @@ impl JoinIrBlockConverter {
|
||||
self.current_instructions.len()
|
||||
);
|
||||
if let Some(block) = mir_func.blocks.get_mut(&self.current_block_id) {
|
||||
let inst_count = self.current_instructions.len();
|
||||
block.instructions = std::mem::take(&mut self.current_instructions);
|
||||
block.instruction_spans = vec![Span::unknown(); inst_count];
|
||||
// Phase 189 FIX: Use extend() instead of assignment to preserve
|
||||
// existing instructions (e.g., PHI from handle_select())
|
||||
let new_insts = std::mem::take(&mut self.current_instructions);
|
||||
let new_count = new_insts.len();
|
||||
block.instructions.extend(new_insts);
|
||||
block.instruction_spans.extend(vec![Span::unknown(); new_count]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -37,6 +37,27 @@ pub fn convert_join_module_to_mir_with_meta(
|
||||
// 2. 基本のMIR変換(Phase 190: modularized converter)
|
||||
let mir_func = JoinIrFunctionConverter::convert_function(join_func)?;
|
||||
|
||||
// Phase 189 DEBUG: Dump MirFunction blocks to check PHI presence
|
||||
eprintln!(
|
||||
"[joinir/meta] MirFunc '{}' has {} blocks after convert_function:",
|
||||
mir_func.signature.name,
|
||||
mir_func.blocks.len()
|
||||
);
|
||||
for (block_id, block) in &mir_func.blocks {
|
||||
let phi_count = block
|
||||
.instructions
|
||||
.iter()
|
||||
.filter(|i| matches!(i, crate::mir::MirInstruction::Phi { .. }))
|
||||
.count();
|
||||
eprintln!(
|
||||
"[joinir/meta] Block {:?}: {} instructions ({} PHI), terminator={:?}",
|
||||
block_id,
|
||||
block.instructions.len(),
|
||||
phi_count,
|
||||
block.terminator.as_ref().map(|t| format!("{:?}", t).chars().take(40).collect::<String>())
|
||||
);
|
||||
}
|
||||
|
||||
// 3. Phase 40-1: if_modified_varsがあればloop exit PHI生成
|
||||
if let Some(m) = meta.get(func_id) {
|
||||
if let Some(if_vars) = &m.if_modified_vars {
|
||||
|
||||
@ -1,3 +1,4 @@
|
||||
use crate::ast::Span;
|
||||
use crate::mir::optimizer::MirOptimizer;
|
||||
use crate::mir::optimizer_stats::OptimizationStats;
|
||||
use crate::mir::{MirInstruction as I, MirModule};
|
||||
@ -15,10 +16,15 @@ pub fn optimize_boxfield_operations(
|
||||
for (_bb_id, block) in &mut function.blocks {
|
||||
let mut changed = 0usize;
|
||||
let mut out: Vec<I> = Vec::with_capacity(block.instructions.len());
|
||||
let mut out_spans: Vec<Span> = Vec::with_capacity(block.instruction_spans.len());
|
||||
let mut i = 0usize;
|
||||
while i < block.instructions.len() {
|
||||
// Get span for current instruction (or unknown if missing)
|
||||
let span_i = block.instruction_spans.get(i).copied().unwrap_or_else(Span::unknown);
|
||||
|
||||
// Look ahead for simple store-followed-by-load on same box/index
|
||||
if i + 1 < block.instructions.len() {
|
||||
let span_i1 = block.instruction_spans.get(i + 1).copied().unwrap_or_else(Span::unknown);
|
||||
match (&block.instructions[i], &block.instructions[i + 1]) {
|
||||
(
|
||||
I::BoxCall {
|
||||
@ -42,10 +48,12 @@ pub fn optimize_boxfield_operations(
|
||||
// Replace the second with Copy from just-stored value
|
||||
let src_val = a1[1];
|
||||
out.push(block.instructions[i].clone());
|
||||
out_spans.push(span_i);
|
||||
out.push(I::Copy {
|
||||
dst: *dst2,
|
||||
src: src_val,
|
||||
});
|
||||
out_spans.push(span_i1);
|
||||
changed += 1;
|
||||
i += 2;
|
||||
continue;
|
||||
@ -55,10 +63,12 @@ pub fn optimize_boxfield_operations(
|
||||
}
|
||||
}
|
||||
out.push(block.instructions[i].clone());
|
||||
out_spans.push(span_i);
|
||||
i += 1;
|
||||
}
|
||||
if changed > 0 {
|
||||
block.instructions = out;
|
||||
block.instruction_spans = out_spans;
|
||||
stats.boxfield_optimizations += changed;
|
||||
}
|
||||
}
|
||||
|
||||
@ -295,6 +295,16 @@ impl MirPrinter {
|
||||
block: &BasicBlock,
|
||||
types: &BTreeMap<ValueId, MirType>,
|
||||
) -> String {
|
||||
// DEBUG: Check span mismatch
|
||||
if block.instructions.len() != block.instruction_spans.len() {
|
||||
eprintln!(
|
||||
"[printer/DEBUG] Block {:?} SPAN MISMATCH: instructions={}, spans={}",
|
||||
block.id,
|
||||
block.instructions.len(),
|
||||
block.instruction_spans.len()
|
||||
);
|
||||
}
|
||||
|
||||
let mut output = String::new();
|
||||
|
||||
// Block header
|
||||
|
||||
Reference in New Issue
Block a user