diff --git a/docs/development/current/main/phases/phase-287/P6-SCAN_PLAN-INTEGRATION-INSTRUCTIONS.md b/docs/development/current/main/phases/phase-287/P6-SCAN_PLAN-INTEGRATION-INSTRUCTIONS.md index 6958a942..1236c0e9 100644 --- a/docs/development/current/main/phases/phase-287/P6-SCAN_PLAN-INTEGRATION-INSTRUCTIONS.md +++ b/docs/development/current/main/phases/phase-287/P6-SCAN_PLAN-INTEGRATION-INSTRUCTIONS.md @@ -1,8 +1,8 @@ # Phase 287 P6: Scan plan 統合（docs-first, 意味論不変の範囲で） **Date**: 2025-12-27 -**Status**: Ready（next） -**Scope**: `merge/instruction_rewriter.rs` の Stage 1（scan）で生成する `RewritePlan` を、Stage 2（plan）へ “同じ意味” で反映するか、または Stage 1 を削除して pipeline を単純化するための判断を docs で固定する。 +**Status**: Completed ✅ +**Scope**: `merge/instruction_rewriter.rs` の Stage 1（scan）を削除して pipeline を 2-stage（Plan→Apply）へ単純化する（意味論不変）。 **Non-goals**: 挙動変更、ログ恒常増加、silent fallback 追加、テスト専用の暫定コード追加 --- @@ -40,6 +40,12 @@ - debug/log の恒常出力差を出さない（既存 `debug` フラグに従う） - pipeline の見通し（責務）を docs と module 構造で補強する +**Decision（2025-12-27）**: Option B を採用する。 + +理由: +- Stage 2（Plan）は boundary/local map/PHI/tailcall/terminator を扱っており、Scan を“本当に使う”には Scan 側に情報を増殖させやすい。 +- Scan が未使用のまま残るのは二重修正の温床なので、先に削除して 2-stage へ収束させるのが最も構造的に安全。 + --- ## 手順（docs-first） diff --git a/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs b/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs index de824180..e049c234 100644 --- a/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs +++ b/src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs @@ -30,7 +30,7 @@ use super::rewriter::rewrite_context::RewriteContext; use super::contract_checks; // Phase 287 P5: Unified import through stages facade (SSOT) -use super::rewriter::stages::{scan_blocks, plan_rewrites, apply_rewrites}; +use super::rewriter::stages::{plan_rewrites, apply_rewrites}; /// Phase 4: Merge ALL functions and rewrite instructions @@ -65,10 +65,9 @@ pub(super) fn merge_and_rewrite( exit_block_id: BasicBlockId, debug: bool, ) -> Result { - // Phase 286C-4: 3-stage pipeline orchestrator - // 1. scan_blocks() - Read-only analysis (identify what to rewrite) - // 2. plan_rewrites() - Generate rewritten blocks (pure transformation) - // 3. apply_rewrites() - Mutate builder (apply changes) + // Phase 286C-4: pipeline orchestrator + // 1. plan_rewrites() - Generate rewritten blocks (pure transformation) + // 2. apply_rewrites() - Mutate builder (apply changes) let trace = trace::trace(); // Only verbose if explicitly requested via debug flag (not env var - causes test failures) @@ -138,36 +137,15 @@ pub(super) fn merge_and_rewrite( ); } - // ===== STAGE 1: SCAN (Read-only analysis) ===== + // ===== STAGE 1: PLAN (Generate rewritten blocks) ===== if debug { log!( true, - "[merge_and_rewrite] Phase 286C-4 Stage 1: Scanning {} functions", - mir_module.functions.len() - ); - } - - let scan_plan = scan_blocks(mir_module, remapper, value_to_func_name, &ctx, debug)?; - - if debug { - log!( - true, - "[merge_and_rewrite] Phase 286C-4: Scan complete - {} tail calls, {} returns", - scan_plan.tail_calls.len(), - scan_plan.return_conversions.len() - ); - } - - // ===== STAGE 2: PLAN (Generate rewritten blocks) ===== - if debug { - log!( - true, - "[merge_and_rewrite] Phase 286C-4 Stage 2: Planning rewrites" + "[merge_and_rewrite] Phase 286C-4 Stage 1: Planning rewrites" ); } let blocks = plan_rewrites( - scan_plan, mir_module, remapper, function_params, @@ -191,11 +169,11 @@ pub(super) fn merge_and_rewrite( contract_checks::verify_carrier_inputs_complete(b, &blocks.carrier_inputs)?; } - // ===== STAGE 3: APPLY (Mutate builder) ===== + // ===== STAGE 2: APPLY (Mutate builder) ===== if debug { log!( true, - "[merge_and_rewrite] Phase 286C-4 Stage 3: Applying rewrites" + "[merge_and_rewrite] Phase 286C-4 Stage 2: Applying rewrites" ); } diff --git a/src/mir/builder/control_flow/joinir/merge/rewriter/mod.rs b/src/mir/builder/control_flow/joinir/merge/rewriter/mod.rs index 4e8994fe..94d9844f 100644 --- a/src/mir/builder/control_flow/joinir/merge/rewriter/mod.rs +++ b/src/mir/builder/control_flow/joinir/merge/rewriter/mod.rs @@ -45,8 +45,7 @@ pub(super) mod tail_call_detector_box; // Phase 286C-2 Step 2: Tail call detecti pub(super) mod terminator; // Phase 260 P0.1 Step 5: Terminator remapping extracted ✅ pub(super) mod type_propagation; // Phase 260 P0.1 Step 4: Type propagation extracted ✅ -// Phase 286C-2.1: 3-stage pipeline (Scan → Plan → Apply) ✅ -pub(super) mod scan_box; // Stage 1: Data structures +// Phase 286C-2.1: pipeline (Plan → Apply) pub(super) mod plan_box; // Stage 2: Data structures pub(super) mod apply_box; // Stage 3: Data structures (stub) pub(super) mod plan_helpers; // Helper functions for plan_rewrites() diff --git a/src/mir/builder/control_flow/joinir/merge/rewriter/plan_box.rs b/src/mir/builder/control_flow/joinir/merge/rewriter/plan_box.rs index a27dc17c..961042ad 100644 --- a/src/mir/builder/control_flow/joinir/merge/rewriter/plan_box.rs +++ b/src/mir/builder/control_flow/joinir/merge/rewriter/plan_box.rs @@ -1,13 +1,10 @@ -//! PlanBox: Pure transformation (Scan → RewrittenBlocks) +//! RewrittenBlocks: Plan stage output (pure transformation) //! -//! Phase 286C-2.1: Second stage of 3-stage pipeline (Scan → Plan → Apply) -//! Generates new blocks and mappings WITHOUT touching the builder. +//! Plan stage generates new blocks and mappings WITHOUT touching the builder. use crate::mir::{BasicBlock, BasicBlockId, ValueId}; use std::collections::BTreeMap; -use super::scan_box::RewritePlan; - /// Rewritten blocks: New blocks, PHI inputs, carrier inputs (ready to apply) #[derive(Debug, Clone)] pub struct RewrittenBlocks { @@ -23,48 +20,3 @@ pub struct RewrittenBlocks { /// Carrier inputs: Map> pub carrier_inputs: BTreeMap>, } - -/// PlanBox: Pure transformation from RewritePlan to RewrittenBlocks -pub struct PlanBox; - -impl PlanBox { - /// Transform a RewritePlan into RewrittenBlocks - /// - /// This is a PURE transformation - no builder mutation, no side effects. - /// Can update RewriteContext but does NOT touch MirBuilder. - /// - /// # Arguments - /// * `plan` - The rewrite plan from ScanBox - /// - /// # Returns - /// * RewrittenBlocks ready to be applied to the builder - pub fn transform(_plan: RewritePlan) -> RewrittenBlocks { - // TODO: Implement transformation logic - // For now, return empty result - RewrittenBlocks { - new_blocks: Vec::new(), - block_replacements: BTreeMap::new(), - phi_inputs: Vec::new(), - carrier_inputs: BTreeMap::new(), - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_plan_box_basic() { - // Basic smoke test - let plan = RewritePlan { - tail_calls: Vec::new(), - return_conversions: Vec::new(), - phi_adjustments: Vec::new(), - parameter_bindings: Vec::new(), - }; - - let blocks = PlanBox::transform(plan); - assert!(blocks.new_blocks.is_empty()); - } -} diff --git a/src/mir/builder/control_flow/joinir/merge/rewriter/scan_box.rs b/src/mir/builder/control_flow/joinir/merge/rewriter/scan_box.rs deleted file mode 100644 index 1c3b78b9..00000000 --- a/src/mir/builder/control_flow/joinir/merge/rewriter/scan_box.rs +++ /dev/null @@ -1,162 +0,0 @@ -//! ScanBox: Read-only scanning for rewrite planning -//! -//! Phase 286C-2.1: First stage of 3-stage pipeline (Scan → Plan → Apply) -//! Identifies what needs to be rewritten WITHOUT mutating any state. - -use crate::mir::{BasicBlock, BasicBlockId, MirInstruction, ValueId}; -use std::collections::BTreeSet; - -/// Rewrite plan: Describes WHAT to do (doesn't touch instructions yet) -#[derive(Debug, Clone)] -pub struct RewritePlan { - /// Tail calls to be converted to parameter bindings + Jump - pub tail_calls: Vec, - - /// Return instructions to convert to Jump to exit block - pub return_conversions: Vec, - - /// PHI adjustments needed (currently empty, for future use) - pub phi_adjustments: Vec, - - /// Parameter bindings to generate for tail calls - pub parameter_bindings: Vec, -} - -/// Information about a tail call to rewrite -#[derive(Debug, Clone)] -pub struct TailCallRewrite { - /// Block containing the tail call - pub block_id: BasicBlockId, - - /// Target function name - pub target_func_name: String, - - /// Target block (entry block of called function) - pub target_block: BasicBlockId, - - /// Remapped arguments to pass - pub args: Vec, - - /// Whether this is a recursive call (same function) - pub is_recursive: bool, - - /// Whether this is a continuation call (k_exit, etc.) - pub is_continuation: bool, - - /// Whether the target is the loop entry function - pub is_loop_entry: bool, -} - -/// Information about a Return instruction to convert -#[derive(Debug, Clone)] -pub struct ReturnConversion { - /// Block containing the Return - pub block_id: BasicBlockId, - - /// Return value (if any) - pub return_value: Option, - - /// Whether this has exit edge args - pub has_exit_edge_args: bool, - - /// Whether to keep Return (non-skippable continuation) - pub should_keep_return: bool, -} - -/// PHI adjustment (placeholder for future use) -#[derive(Debug, Clone)] -pub struct PhiAdjustment { - /// Block containing the PHI - pub block_id: BasicBlockId, - - /// PHI destination - pub phi_dst: ValueId, -} - -/// Parameter binding to generate -#[derive(Debug, Clone)] -pub struct ParameterBinding { - /// Block to insert the binding into - pub block_id: BasicBlockId, - - /// Destination ValueId (parameter) - pub dst: ValueId, - - /// Source ValueId (argument) - pub src: ValueId, -} - -/// ScanBox: Read-only scanning for rewrite planning -pub struct ScanBox; - -impl ScanBox { - /// Scan a block to identify rewrite operations needed - /// - /// This is a READ-ONLY operation - it doesn't mutate any state, - /// just analyzes what needs to be done. - /// - /// # Arguments - /// * `block` - The block to scan - /// * `block_id` - The block's ID - /// * `func_name` - The function containing this block - /// * Additional context parameters (TODO: consolidate into ScanContext) - /// - /// # Returns - /// * Instructions to skip, tail calls found, returns to convert, etc. - pub fn scan_block( - block: &BasicBlock, - block_id: BasicBlockId, - _func_name: &str, - ) -> BlockScanResult { - let skip_instructions = BTreeSet::new(); - let mut found_tail_call = false; - - // Scan instructions for tail calls, returns, etc. - for (idx, inst) in block.instructions.iter().enumerate() { - match inst { - MirInstruction::Call { .. } => { - // Detect tail calls - // TODO: Implement tail call detection logic - found_tail_call = false; // Placeholder - } - MirInstruction::Return { .. } => { - // Mark return for conversion - // TODO: Implement return conversion detection - } - _ => {} - } - } - - BlockScanResult { - skip_instructions, - found_tail_call, - tail_call_target: None, - } - } -} - -/// Result of scanning a single block -#[derive(Debug, Clone)] -pub struct BlockScanResult { - /// Instruction indices to skip during rewriting - pub skip_instructions: BTreeSet, - - /// Whether a tail call was found - pub found_tail_call: bool, - - /// Tail call target (if found) - pub tail_call_target: Option<(BasicBlockId, Vec)>, -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_scan_box_basic() { - // Basic smoke test - will expand with real logic - let block = BasicBlock::new(BasicBlockId(0)); - let result = ScanBox::scan_block(&block, BasicBlockId(0), "test_func"); - assert!(!result.found_tail_call); - } -} diff --git a/src/mir/builder/control_flow/joinir/merge/rewriter/stages/mod.rs b/src/mir/builder/control_flow/joinir/merge/rewriter/stages/mod.rs index 50c5bc67..e496859e 100644 --- a/src/mir/builder/control_flow/joinir/merge/rewriter/stages/mod.rs +++ b/src/mir/builder/control_flow/joinir/merge/rewriter/stages/mod.rs @@ -3,10 +3,9 @@ //! Phase 287 P3: Modularize instruction_rewriter.rs 3-stage pipeline //! Extracted from instruction_rewriter.rs to separate physical files. //! -//! This module contains the three stages of JoinIR instruction rewriting: -//! 1. **Scan**: Read-only analysis to identify what needs rewriting -//! 2. **Plan**: Transform scan plan into concrete rewritten blocks -//! 3. **Apply**: Apply rewritten blocks to MirBuilder +//! This module contains the pipeline stages of JoinIR instruction rewriting: +//! 1. **Plan**: Build rewritten blocks (pure transformation) +//! 2. **Apply**: Apply rewritten blocks to MirBuilder //! //! # Phase 287 P5: Facade Pattern (Re-export SSOT) //! @@ -15,11 +14,9 @@ //! single entry point for the pipeline. // Module declarations (implementation files) -mod scan; mod plan; mod apply; // Phase 287 P5: Re-export stage functions (facade pattern) -pub(in crate::mir::builder::control_flow::joinir::merge) use scan::scan_blocks; pub(in crate::mir::builder::control_flow::joinir::merge) use plan::plan_rewrites; pub(in crate::mir::builder::control_flow::joinir::merge) use apply::apply_rewrites; diff --git a/src/mir/builder/control_flow/joinir/merge/rewriter/stages/plan/mod.rs b/src/mir/builder/control_flow/joinir/merge/rewriter/stages/plan/mod.rs index a3d9e66a..c7dbe958 100644 --- a/src/mir/builder/control_flow/joinir/merge/rewriter/stages/plan/mod.rs +++ b/src/mir/builder/control_flow/joinir/merge/rewriter/stages/plan/mod.rs @@ -15,7 +15,6 @@ use super::super::{ plan_helpers::{build_local_block_map, sync_spans}, rewrite_context::RewriteContext, - scan_box::RewritePlan, plan_box::RewrittenBlocks, helpers::is_skippable_continuation, }; @@ -60,7 +59,6 @@ mod terminator_rewrite; /// # Phase 287 P5: Re-exported through stages/mod.rs /// Access via stages::{plan_rewrites} for unified API. pub(in crate::mir::builder::control_flow::joinir::merge) fn plan_rewrites( - _plan: RewritePlan, mir_module: &MirModule, remapper: &mut JoinIrIdRemapper, function_params: &BTreeMap>, diff --git a/src/mir/builder/control_flow/joinir/merge/rewriter/stages/scan.rs b/src/mir/builder/control_flow/joinir/merge/rewriter/stages/scan.rs deleted file mode 100644 index 663dce6f..00000000 --- a/src/mir/builder/control_flow/joinir/merge/rewriter/stages/scan.rs +++ /dev/null @@ -1,136 +0,0 @@ -//! Stage 1: Scan - Read-only analysis to identify what needs rewriting -//! -//! Phase 287 P3: Extracted from instruction_rewriter.rs::scan_blocks() -//! -//! Scans all blocks and instructions to identify: -//! - Tail calls to convert -//! - Returns to convert to exit jumps -//! - PHI adjustments needed -//! - Parameter bindings to generate -//! -//! This is a READ-ONLY operation - no mutations, just analysis. - -use super::super::{ - rewrite_context::RewriteContext, - scan_box::{RewritePlan, ReturnConversion, TailCallRewrite}, -}; -use super::super::super::trace; -use crate::mir::{MirInstruction, MirModule, ValueId}; -use crate::mir::builder::joinir_id_remapper::JoinIrIdRemapper; -use std::collections::BTreeMap; - -/// Stage 1: Scan - Read-only analysis to identify what needs rewriting -/// -/// Scans all blocks and instructions to identify: -/// - Tail calls to convert -/// - Returns to convert to exit jumps -/// - PHI adjustments needed -/// - Parameter bindings to generate -/// -/// This is a READ-ONLY operation - no mutations, just analysis. -/// -/// # Phase 287 P5: Re-exported through stages/mod.rs -/// Access via stages::{scan_blocks} for unified API. -pub(in crate::mir::builder::control_flow::joinir::merge) fn scan_blocks( - mir_module: &MirModule, - remapper: &JoinIrIdRemapper, - value_to_func_name: &BTreeMap, - ctx: &RewriteContext, - debug: bool, -) -> Result { - let trace = trace::trace(); - // Only verbose if explicitly requested via debug flag (not env var - causes test failures) - let verbose = debug; - macro_rules! log { - ($enabled:expr, $($arg:tt)*) => { - trace.stderr_if(&format!($($arg)*), $enabled); - }; - } - - let mut plan = RewritePlan { - tail_calls: Vec::new(), - return_conversions: Vec::new(), - phi_adjustments: Vec::new(), - parameter_bindings: Vec::new(), - }; - - // Sort functions for deterministic iteration - let mut functions_merge: Vec<_> = mir_module.functions.iter().collect(); - functions_merge.sort_by_key(|(name, _)| name.as_str()); - - for (func_name, func) in functions_merge { - // Sort blocks for deterministic iteration - let mut blocks_merge: Vec<_> = func.blocks.iter().collect(); - blocks_merge.sort_by_key(|(id, _)| id.0); - - for (old_block_id, old_block) in blocks_merge { - let new_block_id = remapper - .get_block(func_name, *old_block_id) - .ok_or_else(|| format!("Block {:?} not found for {}", old_block_id, func_name))?; - - // Scan instructions for tail calls and PHI adjustments - for inst in &old_block.instructions { - // Detect tail calls - if let MirInstruction::Call { func: callee, args, .. } = inst { - if let Some(callee_name) = value_to_func_name.get(callee) { - // Check if this is an intra-module call (tail call candidate) - if let Some(&target_block) = ctx.function_entry_map.get(callee_name) { - let remapped_args: Vec = args - .iter() - .map(|&v| remapper.get_value(v).unwrap_or(v)) - .collect(); - - let is_recursive = callee_name == func_name; - // For now, mark all tail calls as non-continuation (will be refined in plan stage) - let is_continuation = false; - let is_loop_entry = false; // Will be determined in plan stage - - plan.tail_calls.push(TailCallRewrite { - block_id: new_block_id, - target_func_name: callee_name.clone(), - target_block, - args: remapped_args, - is_recursive, - is_continuation, - is_loop_entry, - }); - - log!( - verbose, - "[scan_blocks] Detected tail call: block={:?} target='{}' args={:?}", - new_block_id, callee_name, args - ); - } - } - } - } - - // Check terminator for Return instructions - if let Some(MirInstruction::Return { value }) = &old_block.terminator { - plan.return_conversions.push(ReturnConversion { - block_id: new_block_id, - return_value: *value, - has_exit_edge_args: old_block.edge_args_from_terminator().is_some(), - should_keep_return: false, // Will be determined in plan stage - }); - - log!( - verbose, - "[scan_blocks] Detected return: block={:?} value={:?}", - new_block_id, value - ); - } - } - } - - if debug { - log!( - true, - "[scan_blocks] Plan summary: {} tail calls, {} returns", - plan.tail_calls.len(), - plan.return_conversions.len() - ); - } - - Ok(plan) -}