diff --git a/src/mir/builder.rs b/src/mir/builder.rs index bfc963e5..bef7d3ad 100644 --- a/src/mir/builder.rs +++ b/src/mir/builder.rs @@ -31,6 +31,7 @@ mod fields; // field access/assignment lowering split pub(crate) mod loops; mod ops; mod phi; +mod phi_merge; // Phase 25.1q: Unified PHI merge helper mod if_form; mod control_flow; // thin wrappers to centralize control-flow entrypoints mod lifecycle; // prepare/lower_root/finalize split diff --git a/src/mir/builder/phi.rs b/src/mir/builder/phi.rs index 0fc2402c..3ec9e4fc 100644 --- a/src/mir/builder/phi.rs +++ b/src/mir/builder/phi.rs @@ -20,101 +20,23 @@ impl MirBuilder { else_map_end_opt: &Option>, skip_var: Option<&str>, ) -> Result<(), String> { - // 📦 Phase 25.1q: Conservative PHI Generation using conservative module + // 📦 Phase 25.1q: Use PhiMergeHelper for unified PHI insertion use std::collections::HashSet; - // Use Conservative PHI strategy (Box Theory) let conservative = crate::mir::phi_core::conservative::ConservativeMerge::analyze( pre_if_snapshot, then_map_end, else_map_end_opt, ); - - // Debug trace if enabled - conservative.trace_if_enabled(pre_if_snapshot, then_map_end, else_map_end_opt); - let changed_set: HashSet = conservative.changed_vars.iter().cloned().collect(); - // Generate PHI for all variables (Conservative) - let trace_conservative = std::env::var("NYASH_CONSERVATIVE_PHI_TRACE") - .ok() - .as_deref() - == Some("1"); - - for name in &conservative.all_vars { - if skip_var.map(|s| s == name.as_str()).unwrap_or(false) { - if trace_conservative { - eprintln!("[Conservative PHI] Skipping {}: matches skip_var", name); - } - continue; - } - - // 📦 Phase 25.1q: Use Conservative module for value resolution - let pre_val_opt = pre_if_snapshot.get(name.as_str()).copied(); - let then_v_opt = then_map_end.get(name.as_str()).copied().or(pre_val_opt); - let else_v_opt = else_map_end_opt - .as_ref() - .and_then(|m| m.get(name.as_str()).copied()) - .or(pre_val_opt); - - let (then_v, else_v) = match (then_v_opt, else_v_opt) { - (Some(tv), Some(ev)) => { - if trace_conservative { - eprintln!( - "[Conservative PHI] Generating PHI for {}: then={:?} else={:?}", - name, tv, ev - ); - } - (tv, ev) - } - (Some(tv), None) => { - let undef = crate::mir::builder::emission::constant::emit_void(self); - if trace_conservative { - eprintln!( - "[Conservative PHI] One-branch variable {}: then={:?} else=void({:?})", - name, tv, undef - ); - } - (tv, undef) - } - (None, Some(ev)) => { - let undef = crate::mir::builder::emission::constant::emit_void(self); - if trace_conservative { - eprintln!( - "[Conservative PHI] One-branch variable {}: then=void({:?}) else={:?}", - name, undef, ev - ); - } - (undef, ev) - } - (None, None) => { - if trace_conservative { - eprintln!("[Conservative PHI] Skipping {}: undefined everywhere", name); - } - continue; - } - }; - // フェーズM: 常にPHI命令を使用（no_phi_mode撤廃） - // incoming の predecessor は "実際に merge に遷移してくる出口ブロック" を使用する - let mut inputs: Vec<(super::BasicBlockId, super::ValueId)> = Vec::new(); - if let Some(tp) = then_exit_block_opt { inputs.push((tp, then_v)); } - if let Some(ep) = else_exit_block_opt { inputs.push((ep, else_v)); } - match inputs.len() { - 0 => {} - 1 => { - let (_pred, v) = inputs[0]; - self.variable_map.insert(name.clone(), v); - } - _ => { - if let Some(func) = self.current_function.as_mut() { func.update_cfg(); } - if let (Some(func), Some(cur_bb)) = (&self.current_function, self.current_block) { - crate::mir::phi_core::common::debug_verify_phi_inputs(func, cur_bb, &inputs); - } - let merged = self.insert_phi(inputs)?; - self.variable_map.insert(name.clone(), merged); - } - } - } + // Use PhiMergeHelper for unified variable merging + let mut helper = super::phi_merge::PhiMergeHelper::new( + self, + then_exit_block_opt, + else_exit_block_opt, + ); + helper.merge_all_vars(pre_if_snapshot, then_map_end, else_map_end_opt, skip_var)?; // Ensure pinned synthetic slots ("__pin$...") have a block-local definition at the merge, // even if their values did not change across branches. This avoids undefined uses when diff --git a/src/mir/builder/phi_merge.rs b/src/mir/builder/phi_merge.rs new file mode 100644 index 00000000..dddbb08b --- /dev/null +++ b/src/mir/builder/phi_merge.rs @@ -0,0 +1,238 @@ +//! PHI Merge Helper - Unified PHI insertion logic +//! +//! Phase 25.1q: PhiMergeHelper統一化 +//! - phi.rs の2箇所のPHI insertion重複（L120-137 vs L155-174）を統一 +//! - Conservative戦略によるvariable merging の一元化 +//! +//! Box-First理論: PHI insertion の境界を明確にし、差し替え可能な箱として提供 + +use super::{BasicBlockId, MirBuilder, ValueId}; +use crate::mir::MirInstruction; +use std::collections::HashMap; + +/// PHI Merge Helper - 統一PHI挿入ロジック（Conservative戦略） +/// +/// # Purpose +/// - 複数のブランチ出口からマージブロックへのPHI挿入を統一処理 +/// - Conservative戦略: 全変数に対してPHIを生成（correctness-first） +/// +/// # Usage +/// ```ignore +/// let mut helper = PhiMergeHelper::new(&mut builder, then_exit, else_exit); +/// helper.merge_variable("x".to_string(), then_v, else_v)?; +/// ``` +pub struct PhiMergeHelper<'a> { + builder: &'a mut MirBuilder, + then_exit: Option, + else_exit: Option, +} + +impl<'a> PhiMergeHelper<'a> { + /// Create a new PhiMergeHelper + /// + /// # Arguments + /// * `builder` - MirBuilder instance + /// * `then_exit` - Then-branch exit block (None if unreachable) + /// * `else_exit` - Else-branch exit block (None if unreachable) + pub fn new( + builder: &'a mut MirBuilder, + then_exit: Option, + else_exit: Option, + ) -> Self { + Self { + builder, + then_exit, + else_exit, + } + } + + /// Merge a single variable using Conservative PHI strategy + /// + /// # Arguments + /// * `name` - Variable name + /// * `then_v` - Value from then-branch + /// * `else_v` - Value from else-branch + /// + /// # Returns + /// Ok(()) on success, Err(String) on failure + /// + /// # Conservative Strategy + /// - 0 predecessors: skip (unreachable) + /// - 1 predecessor: direct insert (no PHI needed) + /// - 2+ predecessors: insert PHI node + pub fn merge_variable( + &mut self, + name: String, + then_v: ValueId, + else_v: ValueId, + ) -> Result<(), String> { + let mut inputs: Vec<(BasicBlockId, ValueId)> = Vec::new(); + if let Some(tp) = self.then_exit { + inputs.push((tp, then_v)); + } + if let Some(ep) = self.else_exit { + inputs.push((ep, else_v)); + } + + match inputs.len() { + 0 => { + // Both branches unreachable - skip + Ok(()) + } + 1 => { + // Single predecessor - direct insert (no PHI) + let (_pred, v) = inputs[0]; + self.builder.variable_map.insert(name, v); + Ok(()) + } + _ => { + // Multiple predecessors - insert PHI + if let Some(func) = self.builder.current_function.as_mut() { + func.update_cfg(); + } + if let (Some(func), Some(cur_bb)) = + (&self.builder.current_function, self.builder.current_block) + { + crate::mir::phi_core::common::debug_verify_phi_inputs(func, cur_bb, &inputs); + } + let merged = self.builder.insert_phi(inputs)?; + self.builder.variable_map.insert(name, merged); + Ok(()) + } + } + } + + /// Merge a variable with explicit destination ValueId (for primary result) + /// + /// # Arguments + /// * `dst` - Destination ValueId for PHI result + /// * `then_v` - Value from then-branch + /// * `else_v` - Value from else-branch + /// + /// # Returns + /// Ok(()) on success, Err(String) on failure + pub fn merge_with_dst( + &mut self, + dst: ValueId, + then_v: ValueId, + else_v: ValueId, + ) -> Result<(), String> { + let mut inputs: Vec<(BasicBlockId, ValueId)> = Vec::new(); + if let Some(tp) = self.then_exit { + inputs.push((tp, then_v)); + } + if let Some(ep) = self.else_exit { + inputs.push((ep, else_v)); + } + + match inputs.len() { + 0 | 1 => { + // Should not happen for explicit dst merge + Ok(()) + } + _ => { + // Insert PHI with explicit dst + if let Some(func) = self.builder.current_function.as_mut() { + func.update_cfg(); + } + if let (Some(func), Some(cur_bb)) = + (&self.builder.current_function, self.builder.current_block) + { + crate::mir::phi_core::common::debug_verify_phi_inputs(func, cur_bb, &inputs); + } + self.builder.insert_phi_with_dst(dst, inputs)?; + Ok(()) + } + } + } + + /// Merge all variables from both branches (Conservative strategy) + /// + /// # Arguments + /// * `pre_if_snapshot` - Variable map before if statement + /// * `then_map_end` - Variable map at end of then-branch + /// * `else_map_end_opt` - Variable map at end of else-branch (None for empty else) + /// * `skip_var` - Optional variable name to skip (already merged elsewhere) + /// + /// # Returns + /// Ok(()) on success, Err(String) on failure + pub fn merge_all_vars( + &mut self, + pre_if_snapshot: &HashMap, + then_map_end: &HashMap, + else_map_end_opt: &Option>, + skip_var: Option<&str>, + ) -> Result<(), String> { + // Use Conservative strategy from conservative module + let conservative = crate::mir::phi_core::conservative::ConservativeMerge::analyze( + pre_if_snapshot, + then_map_end, + else_map_end_opt, + ); + + conservative.trace_if_enabled(pre_if_snapshot, then_map_end, else_map_end_opt); + + let trace_conservative = std::env::var("NYASH_CONSERVATIVE_PHI_TRACE") + .ok() + .as_deref() + == Some("1"); + + for name in &conservative.all_vars { + if skip_var.map(|s| s == name.as_str()).unwrap_or(false) { + if trace_conservative { + eprintln!("[Conservative PHI] Skipping {}: matches skip_var", name); + } + continue; + } + + let pre_val_opt = pre_if_snapshot.get(name.as_str()).copied(); + let then_v_opt = then_map_end.get(name.as_str()).copied().or(pre_val_opt); + let else_v_opt = else_map_end_opt + .as_ref() + .and_then(|m| m.get(name.as_str()).copied()) + .or(pre_val_opt); + + let (then_v, else_v) = match (then_v_opt, else_v_opt) { + (Some(tv), Some(ev)) => { + if trace_conservative { + eprintln!( + "[Conservative PHI] Generating PHI for {}: then={:?} else={:?}", + name, tv, ev + ); + } + (tv, ev) + } + (Some(tv), None) => { + let undef = crate::mir::builder::emission::constant::emit_void(self.builder); + if trace_conservative { + eprintln!( + "[Conservative PHI] One-branch variable {}: then={:?} else=void({:?})", + name, tv, undef + ); + } + (tv, undef) + } + (None, Some(ev)) => { + let undef = crate::mir::builder::emission::constant::emit_void(self.builder); + if trace_conservative { + eprintln!( + "[Conservative PHI] One-branch variable {}: then=void({:?}) else={:?}", + name, undef, ev + ); + } + (undef, ev) + } + (None, None) => { + if trace_conservative { + eprintln!("[Conservative PHI] Skipping {}: undefined everywhere", name); + } + continue; + } + }; + + self.merge_variable(name.clone(), then_v, else_v)?; + } + + Ok(()) + } +}