Merge branch 'main' of https://github.com/arnoox/herkos into pr-f3/gvn

Author: metrics

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

@@ -10,36 +10,33 @@
 //! After substitution, the defining instruction becomes dead (single use was
 //! the branch) and is cleaned up by `dead_instrs`.
 
-use super::utils::{build_global_use_count, instr_dest};
+use super::utils::{build_global_const_map, build_global_use_count, instr_dest, is_zero};
 use crate::ir::{BinOp, IrFunction, IrInstr, IrTerminator, IrValue, UnOp, VarId};
 use std::collections::HashMap;
 
 pub fn eliminate(func: &mut IrFunction) {
     loop {
         let global_uses = build_global_use_count(func);
-        if !fold_one(func, &global_uses) {
+        let global_consts = build_global_const_map(func);
+        if !fold_one(func, &global_uses, &global_consts) {
             break;
         }
     }
 }
 
 /// Attempt a single branch fold across the function. Returns `true` if a
 /// change was made.
-fn fold_one(func: &mut IrFunction, global_uses: &HashMap<VarId, usize>) -> bool {
+fn fold_one(
+    func: &mut IrFunction,
+    global_uses: &HashMap<VarId, usize>,
+    global_consts: &HashMap<VarId, IrValue>,
+) -> bool {
     // Build a map of VarId → defining instruction info.
     // We only care about single-use vars defined by Eqz, Ne(x,0), or Eq(x,0).
     let mut var_defs: HashMap<VarId, VarDef> = HashMap::new();
 
-    // Also build a global constant map for checking if an operand is zero.
-    let global_consts = build_global_const_map(func);
-
     for block in &func.blocks {
-        let mut local_consts = global_consts.clone();
         for instr in &block.instructions {
-            if let IrInstr::Const { dest, value } = instr {
-                local_consts.insert(*dest, *value);
-            }
-
             if let Some(dest) = instr_dest(instr) {
                 match instr {
                     IrInstr::UnOp {
@@ -55,9 +52,9 @@ fn fold_one(func: &mut IrFunction, global_uses: &HashMap<VarId, usize>) -> bool
                         rhs,
                         ..
                     } => {
-                        if is_zero(rhs, &local_consts) {
+                        if is_zero(*rhs, global_consts) {
                             var_defs.insert(dest, VarDef::NeZero(*lhs));
-                        } else if is_zero(lhs, &local_consts) {
+                        } else if is_zero(*lhs, global_consts) {
                             var_defs.insert(dest, VarDef::NeZero(*rhs));
                         }
                     }
@@ -67,9 +64,9 @@ fn fold_one(func: &mut IrFunction, global_uses: &HashMap<VarId, usize>) -> bool
                         rhs,
                         ..
                     } => {
-                        if is_zero(rhs, &local_consts) {
+                        if is_zero(*rhs, global_consts) {
                             var_defs.insert(dest, VarDef::EqZero(*lhs));
-                        } else if is_zero(lhs, &local_consts) {
+                        } else if is_zero(*lhs, global_consts) {
                             var_defs.insert(dest, VarDef::EqZero(*rhs));
                         }
                     }
@@ -133,34 +130,6 @@ enum VarDef {
     EqZero(VarId),
 }
 
-fn is_zero(var: &VarId, consts: &HashMap<VarId, IrValue>) -> bool {
-    matches!(
-        consts.get(var),
-        Some(IrValue::I32(0)) | Some(IrValue::I64(0))
-    )
-}
-
-fn build_global_const_map(func: &IrFunction) -> HashMap<VarId, IrValue> {
-    let mut total_defs: HashMap<VarId, usize> = HashMap::new();
-    let mut const_defs: HashMap<VarId, IrValue> = HashMap::new();
-
-    for block in &func.blocks {
-        for instr in &block.instructions {
-            if let Some(dest) = instr_dest(instr) {
-                *total_defs.entry(dest).or_insert(0) += 1;
-                if let IrInstr::Const { dest, value } = instr {
-                    const_defs.insert(*dest, *value);
-                }
-            }
-        }
-    }
-
-    const_defs
-        .into_iter()
-        .filter(|(v, _)| total_defs.get(v).copied().unwrap_or(0) == 1)
-        .collect()
-}
-
 // ── Tests ─────────────────────────────────────────────────────────────────────
 
 #[cfg(test)]

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

@@ -8,7 +8,7 @@
 use crate::ir::{BinOp, IrFunction, IrInstr, IrValue, UnOp, VarId};
 use std::collections::HashMap;
 
-use super::utils::prune_dead_locals;
+use super::utils::{is_commutative, prune_dead_locals};
 
 // ── Value key ────────────────────────────────────────────────────────────────
 
@@ -45,37 +45,6 @@ impl From<IrValue> for ConstKey {
     }
 }
 
-// ── Commutative op detection ─────────────────────────────────────────────────
-
-/// Returns true for operations where `op(a, b) == op(b, a)`.
-fn is_commutative(op: &BinOp) -> bool {
-    matches!(
-        op,
-        BinOp::I32Add
-            | BinOp::I32Mul
-            | BinOp::I32And
-            | BinOp::I32Or
-            | BinOp::I32Xor
-            | BinOp::I32Eq
-            | BinOp::I32Ne
-            | BinOp::I64Add
-            | BinOp::I64Mul
-            | BinOp::I64And
-            | BinOp::I64Or
-            | BinOp::I64Xor
-            | BinOp::I64Eq
-            | BinOp::I64Ne
-            | BinOp::F32Add
-            | BinOp::F32Mul
-            | BinOp::F32Eq
-            | BinOp::F32Ne
-            | BinOp::F64Add
-            | BinOp::F64Mul
-            | BinOp::F64Eq
-            | BinOp::F64Ne
-    )
-}
-
 /// Build a `ValueKey` for a `BinOp`, normalizing operand order for commutative ops.
 fn binop_key(op: BinOp, lhs: VarId, rhs: VarId) -> ValueKey {
     let (lhs, rhs) = if is_commutative(&op) && lhs.0 > rhs.0 {
@@ -97,8 +66,11 @@ pub fn eliminate(func: &mut IrFunction) {
         let mut value_map: HashMap<ValueKey, VarId> = HashMap::new();
 
         for instr in &mut block.instructions {
-            // In strict SSA form each variable is defined exactly once, so there
-            // is no need to invalidate cached CSE entries on redefinition.
+            // This pass runs on lowered (post-phi) IR. While the function is no
+            // longer globally in SSA form, within any single block each BinOp,
+            // UnOp, and Const dest is still defined at most once: phi lowering
+            // only inserts Assigns into predecessor blocks, never into the block
+            // being processed. So value_map entries are never stale.
             match instr {
                 IrInstr::Const { dest, value } => {
                     let key = ValueKey::Const(ConstKey::from(*value));

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

@@ -466,6 +466,37 @@ pub fn is_side_effect_free(instr: &IrInstr) -> bool {
     }
 }
 
+// ── Commutative op detection ─────────────────────────────────────────────────
+
+/// Returns true for operations where `op(a, b) == op(b, a)`.
+pub fn is_commutative(op: &BinOp) -> bool {
+    matches!(
+        op,
+        BinOp::I32Add
+            | BinOp::I32Mul
+            | BinOp::I32And
+            | BinOp::I32Or
+            | BinOp::I32Xor
+            | BinOp::I32Eq
+            | BinOp::I32Ne
+            | BinOp::I64Add
+            | BinOp::I64Mul
+            | BinOp::I64And
+            | BinOp::I64Or
+            | BinOp::I64Xor
+            | BinOp::I64Eq
+            | BinOp::I64Ne
+            | BinOp::F32Add
+            | BinOp::F32Mul
+            | BinOp::F32Eq
+            | BinOp::F32Ne
+            | BinOp::F64Add
+            | BinOp::F64Mul
+            | BinOp::F64Eq
+            | BinOp::F64Ne
+    )
+}
+
 // ── Rewrite terminator block targets ─────────────────────────────────────────
 
 /// Rewrite all block-ID references in a terminator from `old` to `new`.
@@ -495,6 +526,14 @@ pub fn rewrite_terminator_target(term: &mut IrTerminator, old: BlockId, new: Blo
     }
 }
 
+/// Returns `true` if `var` is known to be zero according to `consts`.
+pub fn is_zero(var: VarId, consts: &HashMap<VarId, IrValue>) -> bool {
+    matches!(
+        consts.get(&var),
+        Some(IrValue::I32(0)) | Some(IrValue::I64(0))
+    )
+}
+
 /// Variables with exactly one definition across the function that is a `Const`
 /// instruction. These can be treated as constants in any block that uses them.
 pub fn build_global_const_map(func: &IrFunction) -> HashMap<VarId, IrValue> {

crates/herkos-runtime/src/memory.rs

@@ -34,7 +34,7 @@ impl<const MAX_PAGES: usize> IsolatedMemory<MAX_PAGES> {
     /// # Errors
     /// Returns `ConstructionError::MemoryInitialPagesExceedsMax` if `initial_pages > MAX_PAGES`.
     #[inline(never)]
-    pub fn try_new(initial_pages: usize) -> Result<Self, crate::ConstructionError> {
+    pub const fn try_new(initial_pages: usize) -> Result<Self, crate::ConstructionError> {
         if initial_pages > MAX_PAGES {
             return Err(crate::ConstructionError::MemoryInitialPagesExceedsMax {
                 initial: initial_pages,
@@ -81,13 +81,13 @@ impl<const MAX_PAGES: usize> IsolatedMemory<MAX_PAGES> {
 
     /// Current number of active pages.
     #[inline(always)]
-    pub fn page_count(&self) -> usize {
+    pub const fn page_count(&self) -> usize {
         self.active_pages
     }
 
     /// Current active size in bytes.
     #[inline(always)]
-    pub fn active_size(&self) -> usize {
+    pub const fn active_size(&self) -> usize {
         self.active_pages * PAGE_SIZE
     }
 
@@ -109,7 +109,7 @@ impl<const MAX_PAGES: usize> IsolatedMemory<MAX_PAGES> {
 
     /// Wasm `memory.size` — returns current page count.
     #[inline(always)]
-    pub fn size(&self) -> i32 {
+    pub const fn size(&self) -> i32 {
         self.active_pages as i32
     }
 

crates/herkos-runtime/src/module.rs

@@ -102,7 +102,7 @@ pub struct LibraryModule<G, const TABLE_SIZE: usize> {
 impl<G, const TABLE_SIZE: usize> LibraryModule<G, TABLE_SIZE> {
     /// Create a new library module with the given globals and table.
     #[inline]
-    pub fn new(globals: G, table: Table<TABLE_SIZE>) -> Self {
+    pub const fn new(globals: G, table: Table<TABLE_SIZE>) -> Self {
         Self { globals, table }
     }
 }

crates/herkos-runtime/src/ops.rs

@@ -131,7 +131,7 @@ pub fn i32_div_u(lhs: i32, rhs: i32) -> WasmResult<i32> {
 /// for this case (because the underlying division overflows), so we handle it
 /// with an explicit branch.
 #[inline(never)]
-pub fn i32_rem_s(lhs: i32, rhs: i32) -> WasmResult<i32> {
+pub const fn i32_rem_s(lhs: i32, rhs: i32) -> WasmResult<i32> {
     if rhs == 0 {
         return Err(WasmTrap::DivisionByZero);
     }
@@ -174,7 +174,7 @@ pub fn i64_div_u(lhs: i64, rhs: i64) -> WasmResult<i64> {
 ///
 /// Same special case as `i32_rem_s`: `i64::MIN rem_s -1 = 0` per Wasm spec.
 #[inline(never)]
-pub fn i64_rem_s(lhs: i64, rhs: i64) -> WasmResult<i64> {
+pub const fn i64_rem_s(lhs: i64, rhs: i64) -> WasmResult<i64> {
     if rhs == 0 {
         return Err(WasmTrap::DivisionByZero);
     }
@@ -198,7 +198,7 @@ pub fn i64_rem_u(lhs: i64, rhs: i64) -> WasmResult<i64> {
 
 /// Wasm `f32.min`: propagates NaN (unlike Rust's `f32::min` which ignores it).
 /// Also preserves the Wasm rule `min(-0.0, +0.0) = -0.0`.
-pub fn wasm_min_f32(a: f32, b: f32) -> f32 {
+pub const fn wasm_min_f32(a: f32, b: f32) -> f32 {
     if a.is_nan() || b.is_nan() {
         return f32::NAN;
     }
@@ -213,7 +213,7 @@ pub fn wasm_min_f32(a: f32, b: f32) -> f32 {
 }
 
 /// Wasm `f32.max`: propagates NaN. `max(-0.0, +0.0) = +0.0`.
-pub fn wasm_max_f32(a: f32, b: f32) -> f32 {
+pub const fn wasm_max_f32(a: f32, b: f32) -> f32 {
     if a.is_nan() || b.is_nan() {
         return f32::NAN;
     }
@@ -228,7 +228,7 @@ pub fn wasm_max_f32(a: f32, b: f32) -> f32 {
 }
 
 /// Wasm `f64.min`: propagates NaN. `min(-0.0, +0.0) = -0.0`.
-pub fn wasm_min_f64(a: f64, b: f64) -> f64 {
+pub const fn wasm_min_f64(a: f64, b: f64) -> f64 {
     if a.is_nan() || b.is_nan() {
         return f64::NAN;
     }
@@ -243,7 +243,7 @@ pub fn wasm_min_f64(a: f64, b: f64) -> f64 {
 }
 
 /// Wasm `f64.max`: propagates NaN. `max(-0.0, +0.0) = +0.0`.
-pub fn wasm_max_f64(a: f64, b: f64) -> f64 {
+pub const fn wasm_max_f64(a: f64, b: f64) -> f64 {
     if a.is_nan() || b.is_nan() {
         return f64::NAN;
     }

crates/herkos-runtime/src/table.rs

@@ -45,7 +45,7 @@ impl<const MAX_SIZE: usize> Table<MAX_SIZE> {
     ///
     /// # Errors
     /// Returns `ConstructionError::TableInitialSizeExceedsMax` if `initial_size > MAX_SIZE`.
-    pub fn try_new(initial_size: usize) -> Result<Self, crate::ConstructionError> {
+    pub const fn try_new(initial_size: usize) -> Result<Self, crate::ConstructionError> {
         if initial_size > MAX_SIZE {
             return Err(crate::ConstructionError::TableInitialSizeExceedsMax {
                 initial: initial_size,
@@ -60,7 +60,7 @@ impl<const MAX_SIZE: usize> Table<MAX_SIZE> {
 
     /// Current number of active table slots.
     #[inline(always)]
-    pub fn size(&self) -> usize {
+    pub const fn size(&self) -> usize {
         self.active_size
     }
 

scripts/compare_metrics.py

@@ -238,14 +238,16 @@ def display_html(groups: Dict[str, List[Dict]], output_path: str = "metrics_repo
 
         timestamps = [parse_timestamp(e["timestamp"]).isoformat() for e in entries]
         values = [e["value"] for e in entries]
+        indices = list(range(len(entries)))
 
         fig.add_trace(
             go.Scatter(
-                x=timestamps,
+                x=indices,
                 y=values,
                 mode="lines+markers",
                 name=name,
-                hovertemplate=f"<b>{name}</b><br>Timestamp: %{{x}}<br>Value: %{{y:.2f}}<extra></extra>",
+                customdata=timestamps,
+                hovertemplate=f"<b>{name}</b><br>Timestamp: %{{customdata}}<br>Value: %{{y:.2f}}<extra></extra>",
             ),
             row=row,
             col=col,