Skip to content

Commit 26cfd69

Browse files
author
bench
committed
refactor: split redundancy and loop passes into separate PRs for better isolation and review
1 parent b628c1d commit 26cfd69

1 file changed

Lines changed: 201 additions & 49 deletions

File tree

PR_SPLIT_STATUS.md

Lines changed: 201 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ This document tracks the progress of splitting PR #12 ("feat: introduce new ir o
1616
| A | `pr-a/wasm-float-ops` | Runtime float ops | ✅ Complete | #28 | ✅ Pass | Ready for review |
1717
| D | `pr-d/optimizer-infrastructure` | Optimizer infrastructure | ✅ Complete | #29 | ✅ Pass | Ready for review |
1818
| E | `pr-e/value-optimizations` | Value optimization passes | ✅ Complete | #30 | ✅ Pass | Ready for review |
19-
| F | `pr-f/redundancy-loop-passes` | Redundancy + loop passes | ⚠️ WIP | #31 | ❌ Needs fix | Pattern match errors in utils |
19+
| F1 | `pr-f1/branch-fold` | Branch condition folding | ⚠️ WIP | #31 | ❌ Needs fix | Split from old PR F |
20+
| F2 | `pr-f2/local-cse` | Local common subexpression elimination | ❌ Not started ||| Split from old PR F |
21+
| F3 | `pr-f3/gvn` | Global value numbering | ❌ Not started ||| Split from old PR F |
22+
| F4 | `pr-f4/licm` | Loop invariant code motion | ❌ Not started ||| Split from old PR F, last |
2023
| G | `pr-g/backend-codegen` | Backend + codegen updates | ❌ Not started ||| Planned |
2124

2225
---
@@ -135,45 +138,154 @@ This document tracks the progress of splitting PR #12 ("feat: introduce new ir o
135138

136139
---
137140

138-
### PR F #31: Redundancy Elimination + Loop Passes ⚠️ WIP
141+
### PR F1 #31: Branch Condition Folding ⚠️ WIP
139142

140-
**Status**: WIP — files copied, needs test fixes
143+
**Status**: WIP — file exists on current branch, needs pattern match fixes
141144

142-
**Files** (from PR #12, needs adjustment):
143-
- `crates/herkos-core/src/optimizer/local_cse.rs` (+575, new)
144-
- Local common subexpression elimination within blocks
145-
- `crates/herkos-core/src/optimizer/gvn.rs` (+619, new)
146-
- Global value numbering across blocks using dominator tree
147-
- `crates/herkos-core/src/optimizer/licm.rs` (+1,307, new)
148-
- Loop invariant code motion
145+
**Branch**: `pr-f1/branch-fold` (rename/split from `pr-f/redundancy-loop-passes`)
146+
147+
**Files**:
149148
- `crates/herkos-core/src/optimizer/branch_fold.rs` (+366, new)
150-
- Branch condition simplification
149+
- Simplifies branch conditions: constant branches, always-taken jumps
151150
- `crates/herkos-core/src/optimizer/mod.rs` (updated)
152-
- Registers all 4 passes in `optimize_lowered_ir` pipeline
151+
- Registers `branch_fold::eliminate` in `optimize_lowered_ir` pipeline
153152

154-
**Tests**: ❌ Compile errors — pattern matching incomplete
153+
**Tests**: ❌ Compile errors — missing pattern match arms
155154

156155
**Known Issues**:
157-
1. Missing pattern match arms for `MemoryFill`, `MemoryInit`, `DataDrop` in several functions
158-
- Affects: branch_fold, gvn, licm, local_cse tests
159-
- Fix: Add cases to all `match instr` blocks that pattern match IR instructions
160-
2. Tests reference removed `IrFunction.needs_host` field
161-
- Already fixed for PRs D & E, needs same fix in PR F files
156+
1. Missing pattern match arms for `MemoryFill`, `MemoryInit`, `DataDrop` in test helpers
157+
- Fix: Add exhaustive arms to all `match instr` blocks in branch_fold tests
158+
2. Tests may reference removed `IrFunction.needs_host` field — verify and remove
159+
160+
**Passes Registered** (post-lowering, added to iteration loop):
161+
- `branch_fold::eliminate` (after `dead_instrs`)
162+
163+
**Dependencies**:
164+
- ✅ Requires PR D (optimizer infrastructure + utils)
165+
- ✅ Requires PR E (value optimizations)
166+
167+
**TODO Before Merge**:
168+
- [ ] Create branch `pr-f1/branch-fold` from PR E base
169+
- [ ] Cherry-pick only `branch_fold.rs` + `mod.rs` changes
170+
- [ ] Fix missing IR instruction pattern matches in branch_fold tests
171+
- [ ] Run `cargo test -p herkos-core --lib`
172+
- [ ] Run `cargo clippy` and `cargo fmt --check`
173+
174+
---
175+
176+
### PR F2: Local Common Subexpression Elimination ❌ Not Started
177+
178+
**Status**: Not started — code exists in `pr-f/redundancy-loop-passes`, needs isolation
179+
180+
**Branch**: `pr-f2/local-cse` (to be created from PR F1)
181+
182+
**Files**:
183+
- `crates/herkos-core/src/optimizer/local_cse.rs` (+575, new)
184+
- Eliminates redundant computations within a single basic block
185+
- Keyed on instruction structure (opcode + operands), no cross-block analysis
186+
- `crates/herkos-core/src/optimizer/mod.rs` (updated)
187+
- Registers `local_cse::eliminate` in `optimize_lowered_ir` pipeline
188+
189+
**Tests**: ❌ Compile errors (same pattern match issues as F1)
190+
191+
**Known Issues**:
192+
- Same missing `MemoryFill`, `MemoryInit`, `DataDrop` pattern arms as F1
193+
- Fix: Add exhaustive arms to all `match instr` blocks in local_cse tests
162194

163195
**Passes Registered** (post-lowering, in iteration loop):
164-
- `local_cse``gvn``branch_fold``licm` (with dead_instrs between)
196+
- `local_cse::eliminate` (after structural passes, before GVN)
165197

166198
**Dependencies**:
167199
- ✅ Requires PR D (optimizer infrastructure + utils)
168200
- ✅ Requires PR E (value optimizations)
201+
- ✅ Requires PR F1 (branch fold reduces branches before CSE runs)
202+
203+
**TODO Before Merge**:
204+
- [ ] Create branch `pr-f2/local-cse` from PR F1 base
205+
- [ ] Cherry-pick only `local_cse.rs` + `mod.rs` changes
206+
- [ ] Fix missing IR instruction pattern matches in local_cse tests
207+
- [ ] Run `cargo test -p herkos-core --lib`
208+
- [ ] Run `cargo clippy` and `cargo fmt --check`
209+
210+
---
211+
212+
### PR F3: Global Value Numbering ❌ Not Started
213+
214+
**Status**: Not started — code exists in `pr-f/redundancy-loop-passes`, needs isolation
215+
216+
**Branch**: `pr-f3/gvn` (to be created from PR F2)
217+
218+
**Files**:
219+
- `crates/herkos-core/src/optimizer/gvn.rs` (+619, new)
220+
- Value numbering across basic blocks using dominator tree traversal
221+
- Eliminates redundant computations that `local_cse` cannot catch across blocks
222+
- `crates/herkos-core/src/optimizer/mod.rs` (updated)
223+
- Registers `gvn::eliminate` in `optimize_lowered_ir` pipeline
224+
225+
**Tests**: ❌ Compile errors (same pattern match issues as F1/F2)
226+
227+
**Known Issues**:
228+
- Same missing `MemoryFill`, `MemoryInit`, `DataDrop` pattern arms
229+
- Dominator tree construction correctness should be carefully verified
230+
231+
**Passes Registered** (post-lowering, in iteration loop):
232+
- `gvn::eliminate` (after `local_cse`, before `dead_instrs`)
233+
234+
**Dependencies**:
235+
- ✅ Requires PR D (optimizer infrastructure + utils, including `build_predecessors`)
236+
- ✅ Requires PR E (value optimizations)
237+
- ✅ Requires PR F2 (local CSE runs first, GVN handles cross-block residuals)
238+
239+
**TODO Before Merge**:
240+
- [ ] Create branch `pr-f3/gvn` from PR F2 base
241+
- [ ] Cherry-pick only `gvn.rs` + `mod.rs` changes
242+
- [ ] Fix missing IR instruction pattern matches in gvn tests
243+
- [ ] Verify dominator tree construction correctness
244+
- [ ] Run `cargo test -p herkos-core --lib`
245+
- [ ] Run `cargo clippy` and `cargo fmt --check`
246+
247+
---
248+
249+
### PR F4: Loop Invariant Code Motion ❌ Not Started
250+
251+
**Status**: Not started — code exists in `pr-f/redundancy-loop-passes`, needs isolation
252+
253+
**Branch**: `pr-f4/licm` (to be created from PR F3, **last of the F series**)
254+
255+
**Files**:
256+
- `crates/herkos-core/src/optimizer/licm.rs` (+1,307, new)
257+
- Detects natural loops via back-edges and dominator tree
258+
- Hoists side-effect-free loop-invariant instructions to loop preheaders
259+
- `crates/herkos-core/src/optimizer/mod.rs` (updated)
260+
- Uncomments and registers `licm::eliminate` in `optimize_lowered_ir` pipeline
261+
- Currently commented out: `// licm::eliminate(func);`
262+
263+
**Tests**: ❌ Compile errors (same pattern match issues + licm is currently commented out)
264+
265+
**Known Issues**:
266+
- Same missing `MemoryFill`, `MemoryInit`, `DataDrop` pattern arms
267+
- Loop detection may be conservative — verify back-edge identification
268+
- LICM must not hoist instructions with side effects (memory writes, traps)
269+
270+
**Passes Registered** (post-lowering, end of iteration loop):
271+
- `licm::eliminate` (after GVN and `dead_instrs`, final pass in loop body)
272+
273+
**Dependencies**:
274+
- ✅ Requires PR D (optimizer infrastructure + utils, `is_side_effect_free`)
275+
- ✅ Requires PR E (value optimizations)
276+
- ✅ Requires PR F1 (branch fold first simplifies loop exit conditions)
277+
- ✅ Requires PR F2 (local CSE)
278+
- ✅ Requires PR F3 (GVN — hoisting is most effective after redundancies removed)
169279

170280
**TODO Before Merge**:
171-
- [ ] Fix missing IR instruction pattern matches (MemoryFill, MemoryInit, DataDrop)
172-
- [ ] Remove remaining `needs_host` references in test code
173-
- [ ] Run full test suite: `cargo test -p herkos-core --lib`
174-
- [ ] Verify dominator tree construction in GVN
175-
- [ ] Check LICM loop detection correctness
281+
- [ ] Create branch `pr-f4/licm` from PR F3 base
282+
- [ ] Cherry-pick only `licm.rs` + `mod.rs` changes (uncomment licm call)
283+
- [ ] Fix missing IR instruction pattern matches in licm tests
284+
- [ ] Verify loop detection and preheader insertion correctness
285+
- [ ] Check `is_side_effect_free` covers all hoistable instructions
176286
- [ ] Test with loop-heavy Wasm (e.g., array operations)
287+
- [ ] Run `cargo test -p herkos-core --lib`
288+
- [ ] Run `cargo clippy` and `cargo fmt --check`
177289

178290
---
179291

@@ -227,24 +339,33 @@ main
227339
├─ main + A + D
228340
↓ merge PR E
229341
├─ main + A + D + E
230-
↓ merge PR F (after fixes)
231-
├─ main + A + D + E + F
342+
↓ merge PR F1 (branch fold)
343+
├─ main + A + D + E + F1
344+
↓ merge PR F2 (local CSE)
345+
├─ main + A + D + E + F1 + F2
346+
↓ merge PR F3 (GVN)
347+
├─ main + A + D + E + F1 + F2 + F3
348+
↓ merge PR F4 (LICM — last)
349+
├─ main + A + D + E + F1 + F2 + F3 + F4
232350
↓ merge PR G
233-
└─ main + A + D + E + F + G (complete)
351+
└─ main + A + D + E + F1–F4 + G (complete)
234352
```
235353

236354
### Rationale
237355
- A is independent (pure runtime addition)
238356
- D is infrastructure foundation
239357
- E depends on A (float ops) + D (utilities)
240-
- F depends on E (builds on value passes)
358+
- F1 depends on E — branch fold simplifies control flow for later passes
359+
- F2 depends on F1 — local CSE runs after branches are simplified
360+
- F3 depends on F2 — GVN extends CSE globally across blocks
361+
- F4 depends on F3 — LICM is most effective after redundancies are removed (last pass)
241362
- G depends on A (must have runtime ops to codegen)
242363

243364
---
244365

245366
## Current Blockers
246367

247-
### PR F Compilation Errors
368+
### PR F1–F4 Compilation Errors
248369

249370
**Error Pattern**:
250371
```
@@ -256,7 +377,7 @@ error[E0004]: non-exhaustive patterns: `MemoryFill`, `MemoryInit`, `DataDrop`
256377
**Solution**: Add pattern match arms to all functions in optimizer files:
257378

258379
```rust
259-
// Example fix for for_each_use in utils.rs
380+
// Example fix pattern
260381
IrInstr::MemoryFill { dst, val, len } => {
261382
f(*dst);
262383
f(*val);
@@ -270,13 +391,13 @@ IrInstr::MemoryInit { dst, src_offset, len, .. } => {
270391
IrInstr::DataDrop { .. } => {}
271392
```
272393

273-
**Affected Files in PR F**:
274-
- branch_fold.rs (test code)
275-
- gvn.rs (test code)
276-
- licm.rs (test code)
277-
- local_cse.rs (test code)
394+
**Affected files (fix needed in each isolated PR)**:
395+
- `branch_fold.rs` test code → fix in PR F1
396+
- `local_cse.rs` test code → fix in PR F2
397+
- `gvn.rs` test code → fix in PR F3
398+
- `licm.rs` test code → fix in PR F4
278399

279-
**Action Item**: Apply pattern match fixes and re-run tests before final review.
400+
**Action Item**: Apply pattern match fixes per file when working on each isolated PR branch.
280401

281402
---
282403

@@ -286,7 +407,10 @@ IrInstr::DataDrop { .. } => {}
286407
- [x] PR A: `cargo test -p herkos-runtime` ✅ 121 pass
287408
- [x] PR D: `cargo test -p herkos-core --lib` ✅ 117 pass
288409
- [x] PR E: `cargo test -p herkos-core --lib` ✅ 201 pass
289-
- [ ] PR F: `cargo test -p herkos-core --lib` ❌ Needs fixes
410+
- [ ] PR F1: `cargo test -p herkos-core --lib` ❌ Needs fixes
411+
- [ ] PR F2: `cargo test -p herkos-core --lib` ❌ Needs fixes
412+
- [ ] PR F3: `cargo test -p herkos-core --lib` ❌ Needs fixes
413+
- [ ] PR F4: `cargo test -p herkos-core --lib` ❌ Needs fixes
290414
- [ ] PR G: `cargo test` (all crates) ⏳ Not started
291415

292416
### Integration Tests (After Merge)
@@ -297,7 +421,10 @@ IrInstr::DataDrop { .. } => {}
297421
- [x] PR A: `cargo clippy` ✅ clean
298422
- [x] PR D: `cargo clippy` ✅ clean
299423
- [x] PR E: `cargo clippy` ✅ clean
300-
- [ ] PR F: `cargo clippy` ⏳ Not checked (blocked on compile)
424+
- [ ] PR F1: `cargo clippy` ⏳ Blocked on compile
425+
- [ ] PR F2: `cargo clippy` ⏳ Not started
426+
- [ ] PR F3: `cargo clippy` ⏳ Not started
427+
- [ ] PR F4: `cargo clippy` ⏳ Not started
301428
- [ ] PR G: `cargo clippy` ⏳ Not started
302429

303430
### Format Check (All PRs)
@@ -339,10 +466,10 @@ Optimizer Passes:
339466
│ ├── const_prop.rs [pre-lowering, PR E]
340467
│ ├── algebraic.rs [pre-lowering, PR E]
341468
│ ├── copy_prop.rs [pre + post, PR E]
342-
│ ├── local_cse.rs [post-lowering, PR F]
343-
│ ├── gvn.rs [post-lowering, PR F]
344-
│ ├── licm.rs [post-lowering, PR F]
345-
│ ├── branch_fold.rs [post-lowering, PR F]
469+
│ ├── branch_fold.rs [post-lowering, PR F1]
470+
│ ├── local_cse.rs [post-lowering, PR F2]
471+
│ ├── gvn.rs [post-lowering, PR F3]
472+
│ ├── licm.rs [post-lowering, PR F4]
346473
│ └── mod.rs [pipeline coordination]
347474
348475
Runtime:
@@ -361,11 +488,36 @@ Codegen (PR G):
361488

362489
## Next Steps
363490

364-
### Immediate (Complete PR F)
365-
1. [ ] Fix pattern matches for MemoryFill/MemoryInit/DataDrop in PR F
366-
2. [ ] Run `cargo test -p herkos-core --lib` and confirm all pass
367-
3. [ ] Run `cargo clippy` and `cargo fmt --check`
368-
4. [ ] Push fixes to `pr-f/redundancy-loop-passes`
491+
### Immediate (Create PR F1 — branch fold)
492+
1. [ ] Create branch `pr-f1/branch-fold` from PR E tip
493+
2. [ ] Cherry-pick only `branch_fold.rs` + `mod.rs` (branch_fold registration) from old `pr-f/redundancy-loop-passes`
494+
3. [ ] Fix missing `MemoryFill`/`MemoryInit`/`DataDrop` pattern arms in `branch_fold.rs` tests
495+
4. [ ] Run `cargo test -p herkos-core --lib` — confirm all pass
496+
5. [ ] Run `cargo clippy` and `cargo fmt --check`
497+
6. [ ] Push and open PR against PR E branch (or main if E is merged)
498+
499+
### Next (PR F2 — local CSE)
500+
1. [ ] Create branch `pr-f2/local-cse` from PR F1 tip
501+
2. [ ] Cherry-pick only `local_cse.rs` + `mod.rs` changes
502+
3. [ ] Fix pattern match arms in `local_cse.rs` tests
503+
4. [ ] Run tests, clippy, fmt
504+
5. [ ] Open PR
505+
506+
### Then (PR F3 — GVN)
507+
1. [ ] Create branch `pr-f3/gvn` from PR F2 tip
508+
2. [ ] Cherry-pick only `gvn.rs` + `mod.rs` changes
509+
3. [ ] Fix pattern match arms in `gvn.rs` tests
510+
4. [ ] Verify dominator tree construction
511+
5. [ ] Run tests, clippy, fmt
512+
6. [ ] Open PR
513+
514+
### Then (PR F4 — LICM, last)
515+
1. [ ] Create branch `pr-f4/licm` from PR F3 tip
516+
2. [ ] Cherry-pick only `licm.rs` + `mod.rs` changes (uncomment `licm::eliminate`)
517+
3. [ ] Fix pattern match arms in `licm.rs` tests
518+
4. [ ] Verify loop detection and preheader insertion
519+
5. [ ] Run tests, clippy, fmt
520+
6. [ ] Open PR
369521

370522
### Short Term (Prepare PR G)
371523
1. [ ] Extract codegen changes from PR #12
@@ -390,5 +542,5 @@ Codegen (PR G):
390542

391543
---
392544

393-
*Last updated: 2026-03-23*
394-
*Status: 4 of 5 PRs complete, PR F WIP, PR G planned*
545+
*Last updated: 2026-03-27*
546+
*Status: 3 of 8 PRs complete (A, D, E); PR F split into F1–F4 (one pass each, LICM last); PR G planned*

0 commit comments

Comments
 (0)