Skip to content

Commit edd1a96

Browse files
benchclaude
andcommitted
refactor: move is_zero to utils, deduplicate build_global_const_map
- Remove local `build_global_const_map` from branch_fold — utils already provides an identical implementation; import it instead - Move `is_zero` helper to utils so it can be shared by other passes - Compute both `global_uses` and `global_consts` in `eliminate` and pass both into `fold_one`, making the two maps symmetric at the call site - Drop the per-block `local_consts` clone — `build_global_const_map` already captures all Const-defined variables across the function in SSA form, so the block-level augmentation was redundant Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d8ad3a1 commit edd1a96

2 files changed

Lines changed: 20 additions & 43 deletions

File tree

crates/herkos-core/src/optimizer/branch_fold.rs

Lines changed: 12 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -10,36 +10,33 @@
1010
//! After substitution, the defining instruction becomes dead (single use was
1111
//! the branch) and is cleaned up by `dead_instrs`.
1212
13-
use super::utils::{build_global_use_count, instr_dest};
13+
use super::utils::{build_global_const_map, build_global_use_count, instr_dest, is_zero};
1414
use crate::ir::{BinOp, IrFunction, IrInstr, IrTerminator, IrValue, UnOp, VarId};
1515
use std::collections::HashMap;
1616

1717
pub fn eliminate(func: &mut IrFunction) {
1818
loop {
1919
let global_uses = build_global_use_count(func);
20-
if !fold_one(func, &global_uses) {
20+
let global_consts = build_global_const_map(func);
21+
if !fold_one(func, &global_uses, &global_consts) {
2122
break;
2223
}
2324
}
2425
}
2526

2627
/// Attempt a single branch fold across the function. Returns `true` if a
2728
/// change was made.
28-
fn fold_one(func: &mut IrFunction, global_uses: &HashMap<VarId, usize>) -> bool {
29+
fn fold_one(
30+
func: &mut IrFunction,
31+
global_uses: &HashMap<VarId, usize>,
32+
global_consts: &HashMap<VarId, IrValue>,
33+
) -> bool {
2934
// Build a map of VarId → defining instruction info.
3035
// We only care about single-use vars defined by Eqz, Ne(x,0), or Eq(x,0).
3136
let mut var_defs: HashMap<VarId, VarDef> = HashMap::new();
3237

33-
// Also build a global constant map for checking if an operand is zero.
34-
let global_consts = build_global_const_map(func);
35-
3638
for block in &func.blocks {
37-
let mut local_consts = global_consts.clone();
3839
for instr in &block.instructions {
39-
if let IrInstr::Const { dest, value } = instr {
40-
local_consts.insert(*dest, *value);
41-
}
42-
4340
if let Some(dest) = instr_dest(instr) {
4441
match instr {
4542
IrInstr::UnOp {
@@ -55,9 +52,9 @@ fn fold_one(func: &mut IrFunction, global_uses: &HashMap<VarId, usize>) -> bool
5552
rhs,
5653
..
5754
} => {
58-
if is_zero(rhs, &local_consts) {
55+
if is_zero(*rhs, global_consts) {
5956
var_defs.insert(dest, VarDef::NeZero(*lhs));
60-
} else if is_zero(lhs, &local_consts) {
57+
} else if is_zero(*lhs, global_consts) {
6158
var_defs.insert(dest, VarDef::NeZero(*rhs));
6259
}
6360
}
@@ -67,9 +64,9 @@ fn fold_one(func: &mut IrFunction, global_uses: &HashMap<VarId, usize>) -> bool
6764
rhs,
6865
..
6966
} => {
70-
if is_zero(rhs, &local_consts) {
67+
if is_zero(*rhs, global_consts) {
7168
var_defs.insert(dest, VarDef::EqZero(*lhs));
72-
} else if is_zero(lhs, &local_consts) {
69+
} else if is_zero(*lhs, global_consts) {
7370
var_defs.insert(dest, VarDef::EqZero(*rhs));
7471
}
7572
}
@@ -133,34 +130,6 @@ enum VarDef {
133130
EqZero(VarId),
134131
}
135132

136-
fn is_zero(var: &VarId, consts: &HashMap<VarId, IrValue>) -> bool {
137-
matches!(
138-
consts.get(var),
139-
Some(IrValue::I32(0)) | Some(IrValue::I64(0))
140-
)
141-
}
142-
143-
fn build_global_const_map(func: &IrFunction) -> HashMap<VarId, IrValue> {
144-
let mut total_defs: HashMap<VarId, usize> = HashMap::new();
145-
let mut const_defs: HashMap<VarId, IrValue> = HashMap::new();
146-
147-
for block in &func.blocks {
148-
for instr in &block.instructions {
149-
if let Some(dest) = instr_dest(instr) {
150-
*total_defs.entry(dest).or_insert(0) += 1;
151-
if let IrInstr::Const { dest, value } = instr {
152-
const_defs.insert(*dest, *value);
153-
}
154-
}
155-
}
156-
}
157-
158-
const_defs
159-
.into_iter()
160-
.filter(|(v, _)| total_defs.get(v).copied().unwrap_or(0) == 1)
161-
.collect()
162-
}
163-
164133
// ── Tests ─────────────────────────────────────────────────────────────────────
165134

166135
#[cfg(test)]

crates/herkos-core/src/optimizer/utils.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,14 @@ pub fn rewrite_terminator_target(term: &mut IrTerminator, old: BlockId, new: Blo
495495
}
496496
}
497497

498+
/// Returns `true` if `var` is known to be zero according to `consts`.
499+
pub fn is_zero(var: VarId, consts: &HashMap<VarId, IrValue>) -> bool {
500+
matches!(
501+
consts.get(&var),
502+
Some(IrValue::I32(0)) | Some(IrValue::I64(0))
503+
)
504+
}
505+
498506
/// Variables with exactly one definition across the function that is a `Const`
499507
/// instruction. These can be treated as constants in any block that uses them.
500508
pub fn build_global_const_map(func: &IrFunction) -> HashMap<VarId, IrValue> {

0 commit comments

Comments
 (0)