Unifies initialization and conversion logic across all 4 loop patterns, eliminating code duplication and establishing single source of truth. ## Changes ### Infrastructure (New) - CommonPatternInitializer (117 lines): Unified loop var extraction + CarrierInfo building - JoinIRConversionPipeline (127 lines): Unified JoinIR→MIR→Merge flow ### Pattern Refactoring - Pattern 1: Uses CommonPatternInitializer + JoinIRConversionPipeline (-25 lines) - Pattern 2: Uses CommonPatternInitializer + JoinIRConversionPipeline (-25 lines) - Pattern 3: Uses CommonPatternInitializer + JoinIRConversionPipeline (-25 lines) - Pattern 4: Uses CommonPatternInitializer + JoinIRConversionPipeline (-40 lines) ### Code Reduction - Total reduction: ~115 lines across all patterns - Zero code duplication in initialization/conversion - Pattern files: 806 lines total (down from ~920) ### Quality Improvements - Single source of truth for initialization - Consistent conversion flow across all patterns - Guaranteed boundary.loop_var_name setting (prevents SSA-undef bugs) - Improved maintainability and testability ### Testing - All 4 patterns tested and passing: - Pattern 1 (Simple While): ✅ - Pattern 2 (With Break): ✅ - Pattern 3 (If-Else PHI): ✅ - Pattern 4 (With Continue): ✅ ### Documentation - Phase 33-22 inventory and results document - Updated joinir-architecture-overview.md with new infrastructure ## Breaking Changes None - pure refactoring with no API changes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
373 lines
11 KiB
Markdown
373 lines
11 KiB
Markdown
# Phase 33-19/33-21完了時点での箱化・モジュール化・レガシー削除・共通化の機会調査
|
||
|
||
**調査日**: 2025-12-07
|
||
**調査範囲**: Phase 33-21 (Parameter remapping fix) 完了後
|
||
**調査目的**: 箱化モジュール化、レガシー削除、共通化の改善機会を発見
|
||
|
||
---
|
||
|
||
## エグゼクティブサマリー
|
||
|
||
### 🎯 主要発見
|
||
|
||
1. **高優先度**: Pattern 1-4で共通する初期化フローの重複(4箇所×約50行 = **200行削減可能**)
|
||
2. **中優先度**: Phase 33-16時代のFallbackロジック(merge/mod.rs:277-307)の必要性検証
|
||
3. **低優先度**: condition_to_joinirとBoolExprLowererの役割分担は適切(削除不要)
|
||
|
||
### 📊 コード規模
|
||
|
||
| モジュール | ファイル数 | 総行数 | 備考 |
|
||
|-----------|----------|-------|-----|
|
||
| patterns/ | 8 | 1,801行 | Pattern 1-4 + 共通モジュール |
|
||
| merge/ | 9 | 1,850行 | JoinIR→MIR変換 |
|
||
| lowering/ | 36 | 10,620行 | JoinIR生成・解析 |
|
||
|
||
---
|
||
|
||
## 推奨改善案
|
||
|
||
### 🔥 高優先度(簡単+インパクト大)
|
||
|
||
#### 1. **CommonPatternInitializer箱の作成**
|
||
|
||
**問題**: 全パターン(Pattern 1-4)で同じ初期化コードが重複
|
||
|
||
**重複コード例**:
|
||
```rust
|
||
// Pattern 1, 2, 3, 4 全てで同じコード
|
||
let loop_var_name = self.extract_loop_variable_from_condition(condition)?;
|
||
let loop_var_id = self
|
||
.variable_map
|
||
.get(&loop_var_name)
|
||
.copied()
|
||
.ok_or_else(|| {
|
||
format!("[cf_loop/patternN] Loop variable '{}' not found", loop_var_name)
|
||
})?;
|
||
|
||
trace::trace().varmap("patternN_start", &self.variable_map);
|
||
```
|
||
|
||
**提案実装**:
|
||
```rust
|
||
// src/mir/builder/control_flow/joinir/patterns/common_init.rs
|
||
pub struct PatternInitializer;
|
||
|
||
impl PatternInitializer {
|
||
/// 全パターン共通の初期化処理
|
||
pub fn extract_loop_context(
|
||
builder: &MirBuilder,
|
||
condition: &ASTNode,
|
||
pattern_name: &str,
|
||
) -> Result<LoopContext, String> {
|
||
let loop_var_name = builder.extract_loop_variable_from_condition(condition)?;
|
||
let loop_var_id = builder.variable_map
|
||
.get(&loop_var_name)
|
||
.copied()
|
||
.ok_or_else(|| {
|
||
format!("[cf_loop/{}] Loop variable '{}' not found",
|
||
pattern_name, loop_var_name)
|
||
})?;
|
||
|
||
trace::trace().varmap(&format!("{}_start", pattern_name), &builder.variable_map);
|
||
|
||
Ok(LoopContext {
|
||
loop_var_name,
|
||
loop_var_id,
|
||
})
|
||
}
|
||
}
|
||
```
|
||
|
||
**削減見込み**:
|
||
- Pattern 1-4各50行 × 4パターン = **200行削減**
|
||
- pattern1_minimal.rs: 176 → 126行(28%削減)
|
||
- pattern2_with_break.rs: 219 → 169行(23%削減)
|
||
- pattern3_with_if_phi.rs: 165 → 115行(30%削減)
|
||
- pattern4_with_continue.rs: 343 → 293行(15%削減)
|
||
|
||
**実装工数**: < 1時間
|
||
|
||
**テスト計画**:
|
||
```bash
|
||
# 全パターンテスト実行
|
||
cargo test --release loop_min_while # Pattern 1
|
||
cargo test --release loop_with_break # Pattern 2
|
||
cargo test --release loop_with_if_phi_sum # Pattern 3
|
||
cargo test --release loop_with_continue # Pattern 4
|
||
```
|
||
|
||
---
|
||
|
||
#### 2. **JoinIR変換パイプライン箱化**
|
||
|
||
**問題**: `convert_join_module_to_mir_with_meta` + `merge_joinir_mir_blocks` の組み合わせが4箇所で重複
|
||
|
||
**重複パターン**:
|
||
```rust
|
||
// Pattern 1, 2, 3, 4 全てで同じフロー
|
||
let mir_module = convert_join_module_to_mir_with_meta(&join_module, &empty_meta)?;
|
||
trace::trace().joinir_stats("patternN", join_module.functions.len(), mir_module.blocks.len());
|
||
let boundary = JoinInlineBoundary::new_inputs_only(...);
|
||
let result = self.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)?;
|
||
```
|
||
|
||
**提案実装**:
|
||
```rust
|
||
// src/mir/builder/control_flow/joinir/patterns/conversion_pipeline.rs
|
||
pub struct JoinIRConversionPipeline;
|
||
|
||
impl JoinIRConversionPipeline {
|
||
/// JoinModule → MIR変換 + マージの統一パイプライン
|
||
pub fn convert_and_merge(
|
||
builder: &mut MirBuilder,
|
||
join_module: JoinModule,
|
||
boundary: JoinInlineBoundary,
|
||
pattern_name: &str,
|
||
debug: bool,
|
||
) -> Result<Option<ValueId>, String> {
|
||
let empty_meta = BTreeMap::new();
|
||
let mir_module = convert_join_module_to_mir_with_meta(&join_module, &empty_meta)
|
||
.map_err(|e| format!("[cf_loop/joinir/{}] MIR conversion failed: {:?}", pattern_name, e))?;
|
||
|
||
trace::trace().joinir_stats(
|
||
pattern_name,
|
||
join_module.functions.len(),
|
||
mir_module.blocks.len(),
|
||
);
|
||
|
||
builder.merge_joinir_mir_blocks(&mir_module, Some(&boundary), debug)
|
||
}
|
||
}
|
||
```
|
||
|
||
**削減見込み**:
|
||
- Pattern 1-4各30行 × 4パターン = **120行削減**
|
||
|
||
**実装工数**: < 1時間
|
||
|
||
---
|
||
|
||
### ⚠️ 中優先度(検証必要)
|
||
|
||
#### 3. **Legacy Fallback削除(merge/mod.rs:277-307)**
|
||
|
||
**場所**: `src/mir/builder/control_flow/joinir/merge/mod.rs:277-307`
|
||
|
||
**問題のコード**:
|
||
```rust
|
||
if function_params.get(main_func_name).is_none() && function_params.get(loop_step_func_name).is_none() {
|
||
// Fallback: Use old behavior (ValueId(0), ValueId(1), ...)
|
||
// This handles patterns that don't have loop_step function
|
||
if let Some(phi_dst) = phi_info.get_carrier_phi(loop_var_name) {
|
||
remapper.set_value(ValueId(0), phi_dst);
|
||
if debug {
|
||
eprintln!(
|
||
"[cf_loop/joinir] Phase 33-16 fallback: Override remap ValueId(0) → {:?} (PHI dst)",
|
||
phi_dst
|
||
);
|
||
}
|
||
}
|
||
for (idx, (carrier_name, entry)) in phi_info.carrier_phis.iter().enumerate() {
|
||
if carrier_name == loop_var_name {
|
||
continue;
|
||
}
|
||
let join_value_id = ValueId(idx as u32);
|
||
remapper.set_value(join_value_id, entry.phi_dst);
|
||
// ...
|
||
}
|
||
}
|
||
```
|
||
|
||
**検証方法**:
|
||
```bash
|
||
# Step 1: Fallbackコードをコメントアウト
|
||
# Step 2: 全パターンテスト実行
|
||
cargo test --release loop_min_while loop_with_break loop_with_if_phi_sum loop_with_continue
|
||
|
||
# Step 3: もしテスト全てPASSなら削除してOK
|
||
```
|
||
|
||
**判定基準**:
|
||
- ✅ テスト全てPASS → **31行削減** + コード複雑度低減
|
||
- ❌ テスト失敗 → Fallback必要(理由をコメントに追記)
|
||
|
||
**実装工数**: < 30分(検証のみ)
|
||
|
||
---
|
||
|
||
### 📘 低優先度(現状維持推奨)
|
||
|
||
#### 4. **condition_to_joinirとBoolExprLowererの役割分担**
|
||
|
||
**調査結果**: **削除不要・重複なし**
|
||
|
||
**理由**:
|
||
1. **明確な責務分離**:
|
||
- `BoolExprLowerer`: AST → MIR(通常の制御フロー用)
|
||
- `condition_to_joinir`: AST → JoinIR(ループパターン用)
|
||
|
||
2. **出力空間が異なる**:
|
||
- BoolExprLowerer: MIR命令(builder経由で状態変更)
|
||
- condition_to_joinir: JoinIR命令(純粋関数変換)
|
||
|
||
3. **使用箇所**:
|
||
- condition_to_joinir: 21箇所(loop lowering専用)
|
||
- BoolExprLowerer: 14箇所(if/while等の通常制御フロー)
|
||
|
||
**コメント改善推奨**:
|
||
```rust
|
||
// src/mir/join_ir/lowering/condition_to_joinir.rs:1
|
||
//! Phase 169: JoinIR Condition Lowering Helper
|
||
//!
|
||
//! **Design Decision (Phase 33-21確認済み)**:
|
||
//! このモジュールはBoolExprLowererと**意図的に別実装**です。
|
||
//! - BoolExprLowerer: MIR空間(状態変更あり)
|
||
//! - condition_to_joinir: JoinIR空間(純粋関数)
|
||
//!
|
||
//! 統合しないでください。
|
||
```
|
||
|
||
---
|
||
|
||
### 🔍 その他の発見
|
||
|
||
#### 5. **LoopUpdateAnalyzer + ContinueBranchNormalizer の統合可能性**
|
||
|
||
**現状**:
|
||
- `LoopUpdateAnalyzer`: Pattern 4のみ使用(1箇所)
|
||
- `ContinueBranchNormalizer`: Pattern 4のみ使用(1箇所)
|
||
|
||
**統合提案**(低優先度):
|
||
```rust
|
||
// src/mir/join_ir/lowering/pattern4_pipeline.rs
|
||
pub struct Pattern4Pipeline;
|
||
|
||
impl Pattern4Pipeline {
|
||
/// Pattern 4専用のAST正規化→解析パイプライン
|
||
pub fn prepare_loop_body(
|
||
body: &[ASTNode],
|
||
carriers: &[CarrierVar],
|
||
) -> (Vec<ASTNode>, HashMap<String, UpdateExpr>) {
|
||
// Step 1: Continue branch正規化
|
||
let normalized_body = ContinueBranchNormalizer::normalize_loop_body(body);
|
||
|
||
// Step 2: Carrier update解析
|
||
let carrier_updates = LoopUpdateAnalyzer::analyze_carrier_updates(&normalized_body, carriers);
|
||
|
||
(normalized_body, carrier_updates)
|
||
}
|
||
}
|
||
```
|
||
|
||
**削減見込み**: コード削減なし、可読性向上のみ
|
||
**実装工数**: < 30分
|
||
**優先度**: 低(Pattern 5/6実装時に再検討)
|
||
|
||
---
|
||
|
||
#### 6. **未使用警告の整理**
|
||
|
||
**発見箇所**:
|
||
```
|
||
warning: methods `detect_from_features`, `detect_with_carrier_name` are never used
|
||
--> src/mir/loop_pattern_detection.rs:106:12
|
||
|
||
warning: methods `exit_analysis` and `has_progress_carrier` are never used
|
||
--> src/mir/join_ir/lowering/loop_scope_shape/structural.rs:84:12
|
||
```
|
||
|
||
**対処方針**:
|
||
- Phase 170以降で使用予定 → `#[allow(dead_code)]` 追加
|
||
- 本当に不要 → 削除(Phase 195確認推奨)
|
||
|
||
---
|
||
|
||
## 実装優先順位
|
||
|
||
### 即座に実装すべき(< 2時間)
|
||
|
||
1. ✅ **CommonPatternInitializer箱化** (1時間)
|
||
- 削減: 200行
|
||
- 効果: Pattern 1-4の保守性向上
|
||
|
||
2. ✅ **JoinIRConversionPipeline箱化** (1時間)
|
||
- 削減: 120行
|
||
- 効果: 変換フロー統一化
|
||
|
||
### 検証後に判断(< 1時間)
|
||
|
||
3. ⚠️ **Legacy Fallback削除検証** (30分)
|
||
- 削減: 31行(削除可能な場合)
|
||
- 条件: テスト全てPASS
|
||
|
||
### Phase 195以降で再検討
|
||
|
||
4. 📘 **Pattern4Pipeline統合** (30分)
|
||
- 削減: なし
|
||
- 効果: 可読性向上
|
||
|
||
5. 📘 **未使用警告整理** (15分)
|
||
- 削減: 不明
|
||
- 効果: コンパイル警告削減
|
||
|
||
---
|
||
|
||
## テスト計画
|
||
|
||
### 退行検証(必須)
|
||
|
||
```bash
|
||
# Pattern 1-4 全体テスト
|
||
cargo test --release loop_min_while # Pattern 1
|
||
cargo test --release loop_with_break # Pattern 2
|
||
cargo test --release loop_with_if_phi_sum # Pattern 3
|
||
cargo test --release loop_with_continue # Pattern 4
|
||
|
||
# SSA-undefエラーチェック
|
||
cargo test --release 2>&1 | grep -i "ssa-undef\|undefined"
|
||
|
||
# WARNING ログチェック
|
||
cargo build --release 2>&1 | grep -i "warning.*unused"
|
||
```
|
||
|
||
### 新規エラー検出
|
||
|
||
```bash
|
||
# Phase 33-21完了時点でのベースライン取得
|
||
cargo test --release 2>&1 | tee /tmp/phase33-21-baseline.log
|
||
|
||
# 改善後の差分確認
|
||
cargo test --release 2>&1 | diff /tmp/phase33-21-baseline.log -
|
||
```
|
||
|
||
---
|
||
|
||
## 期待される効果
|
||
|
||
### 削減見込み
|
||
|
||
| 改善項目 | 削減行数 | 保守性向上 | 実装工数 |
|
||
|---------|---------|-----------|---------|
|
||
| CommonPatternInitializer | 200行 | ★★★★★ | 1時間 |
|
||
| JoinIRConversionPipeline | 120行 | ★★★★☆ | 1時間 |
|
||
| Legacy Fallback削除 | 31行 | ★★★☆☆ | 30分 |
|
||
| **合計** | **351行** | - | **2.5時間** |
|
||
|
||
### コード品質向上
|
||
|
||
1. **DRY原則適用**: Pattern 1-4の重複コード完全削除
|
||
2. **単一責任**: 初期化ロジックが1箇所に集約
|
||
3. **テスト容易性**: CommonPatternInitializerを独立してテスト可能
|
||
4. **拡張性**: Pattern 5/6追加時も同じ箱を使用可能
|
||
|
||
---
|
||
|
||
## 結論
|
||
|
||
Phase 33-19/33-21完了時点で、**351行削減可能な改善機会**を発見。
|
||
特に「CommonPatternInitializer箱化」は高効果・低コストで即座に実装推奨。
|
||
|
||
Legacy Fallback削除は**テスト検証必須**(削除して問題ないか確認)。
|
||
|
||
condition_to_joinirとBoolExprLowererの統合は**不要**(設計上正しい分離)。
|