From e2d061d11339448f2730b5188d454baf6e6eab6f Mon Sep 17 00:00:00 2001 From: nyash-codex Date: Mon, 17 Nov 2025 06:31:31 +0900 Subject: [PATCH] =?UTF-8?q?fix(loop/phi):=20loop=20header=20pinned=20recei?= =?UTF-8?q?ver=20PHI=20=E3=81=AE=E6=9C=AA=E5=AE=9A=E7=BE=A9ValueId?= =?UTF-8?q?=E8=A7=A3=E6=B6=88?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **問題**: TestArgs.process/1 で `Invalid value: use of undefined value ValueId(14)` - loop条件でのmethod call（args.length()）がpin_to_slotで pinned receiver作成 - header PHIが未定義のValueIdを参照していた **根本原因**: - pinned変数のheader PHI作成時、`preheader_value = value` として header blockで作成された値を使用 - 正しくは preheader block の元値を参照すべき **修正内容**: 1. **find_copy_source ヘルパー追加** (src/mir/loop_builder.rs:50-80) - Copy命令を遡ってpreheaderの元値を特定 - NYASH_LOOP_TRACE=1 でデバッグ出力 2. **pinned変数PHI作成ロジック強化** (lines 368-410) - NEW pinned変数: find_copy_source()で正しいpreheader値取得 - INHERITED pinned変数: pre_vars_snapshot から値取得 - PHI inputs に正しい preheader_value を設定 3. **LoopFormOps::new_value修正** (lines 1122-1127) - value_gen.next() → next_value_id() に統一 - 関数ローカルアロケーター使用でValueId整合性確保 4. **next_value_id可視性拡大** (src/mir/builder/utils.rs:33) - pub(super) → pub(crate) でループビルダーから使用可能に **テスト結果**: ✅ Test1 (Direct VM): PASS ✅ Test3 (MIR verify): PASS ⚠️ Test2 (Stage-B): ValueId(17)はif-block PHI問題（別issue） **残存問題**: - Stage-B の ValueId(17) はループ前のif-blockで発生 - if-block PHI reassignment (%0 = phi [%2, bb3]) の構造的問題 - 別タスクで対処予定 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/mir/builder/utils.rs | 2 +- src/mir/loop_builder.rs | 66 ++++++++++++++++++++++++++++++++++------ 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/mir/builder/utils.rs b/src/mir/builder/utils.rs index 11a7a870..db908697 100644 --- a/src/mir/builder/utils.rs +++ b/src/mir/builder/utils.rs @@ -30,7 +30,7 @@ impl super::MirBuilder { /// - Inside function: uses function-local allocator /// - Outside function: uses module-global allocator #[inline] - pub(super) fn next_value_id(&mut self) -> super::ValueId { + pub(crate) fn next_value_id(&mut self) -> super::ValueId { if let Some(ref mut f) = self.current_function { f.next_value_id() // Function context } else { diff --git a/src/mir/loop_builder.rs b/src/mir/loop_builder.rs index acae56f2..fe3723dc 100644 --- a/src/mir/loop_builder.rs +++ b/src/mir/loop_builder.rs @@ -46,7 +46,39 @@ pub struct LoopBuilder<'a> { impl<'a> LoopBuilder<'a> { // Implement phi_core LoopPhiOps on LoopBuilder for in-place delegation - + + /// Find the source value of a Copy instruction in a given block + /// If `dst` is defined by a Copy instruction `dst = copy src`, return Some(src) + /// Otherwise return None + fn find_copy_source(&self, block_id: BasicBlockId, dst: ValueId) -> Option { + let func = self.parent_builder.current_function.as_ref()?; + let block = func.blocks.get(&block_id)?; + + let trace = std::env::var("NYASH_LOOP_TRACE").ok().as_deref() == Some("1"); + if trace { + eprintln!("[loop/copy-trace] searching for dst={:?} in block={:?}, {} instructions", + dst, block_id, block.instructions.len()); + } + + for (idx, inst) in block.instructions.iter().enumerate() { + if let MirInstruction::Copy { dst: inst_dst, src } = inst { + if trace { + eprintln!("[loop/copy-trace] inst#{}: %{} = copy %{}", idx, inst_dst.0, src.0); + } + if *inst_dst == dst { + if trace { + eprintln!("[loop/copy-trace] FOUND! dst={:?} src={:?}", dst, src); + } + return Some(*src); + } + } + } + if trace { + eprintln!("[loop/copy-trace] NOT FOUND for dst={:?}", dst); + } + None + } + // ============================================================= // Control Helpers — break/continue/jumps/unreachable handling // ============================================================= @@ -336,22 +368,36 @@ impl<'a> LoopBuilder<'a> { // When method calls occur in loop conditions (e.g., i < args.length()), pin_to_slot creates // pinned receiver variables like __pin$*@recv. These must have PHI nodes in the loop header. let post_cond_vars = self.get_current_variable_map(); - let mut new_pinned_vars: Vec<(String, ValueId)> = Vec::new(); + let mut new_pinned_vars: Vec<(String, ValueId, ValueId)> = Vec::new(); + if trace { + eprintln!("[loop] post_cond_vars has {} entries", post_cond_vars.len()); + } for (name, &value) in post_cond_vars.iter() { if !name.starts_with("__pin$") { continue; } // Check if this pinned variable existed before condition compilation let was_in_incs = incs.iter().any(|inc| inc.var_name == *name); - if !was_in_incs { - // This is a new pinned variable created during condition evaluation - new_pinned_vars.push((name.clone(), value)); + let was_in_preheader = pre_vars_snapshot.contains_key(name); + if !was_in_incs && !was_in_preheader { + // This is a NEW pinned variable created during condition evaluation (not inherited from preheader) + // We need to find the source of the copy instruction that created this value + let preheader_value = self.find_copy_source(header_id, value).unwrap_or(value); + if trace { + eprintln!("[loop] NEW pinned var: {} value={:?} preheader={:?}", name, value, preheader_value); + } + new_pinned_vars.push((name.clone(), value, preheader_value)); + } else if !was_in_incs && was_in_preheader { + // This pinned variable existed in preheader, so it needs a PHI but we should use the preheader value + let preheader_value = pre_vars_snapshot.get(name).copied().unwrap_or(value); + if trace { + eprintln!("[loop] INHERITED pinned var: {} value={:?} preheader={:?}", name, value, preheader_value); + } + new_pinned_vars.push((name.clone(), value, preheader_value)); } } // Add PHI nodes for new pinned variables in header block - for (name, value) in new_pinned_vars { + for (name, value, preheader_value) in new_pinned_vars { let phi_id = self.new_value(); - // Get the value from preheader (same as the current value since it was just created) - let preheader_value = value; self.emit_phi_at_block_start(header_id, phi_id, vec![(preheader_id, preheader_value)])?; // Update variable map to use PHI value self.update_variable(name.clone(), phi_id); @@ -1076,7 +1122,9 @@ impl crate::mir::phi_core::loop_phi::LoopPhiOps for LoopBuilder<'_> { // Implement LoopFormOps trait for LoopBuilder to support LoopFormBuilder integration impl<'a> LoopFormOps for LoopBuilder<'a> { fn new_value(&mut self) -> ValueId { - self.parent_builder.value_gen.next() + // Use function-local allocator via MirBuilder helper to keep + // ValueId ranges consistent with the current function's value_count. + self.parent_builder.next_value_id() } fn is_parameter(&self, name: &str) -> bool {