Merge pull request #3157 from veryl-lang/fix/wave-vs-aot-c-localize

fix(simulator): waveform dumping turns comb localization off

Author: dalance

crates/simulator/src/backend/aot_c.rs

@@ -13,6 +13,20 @@ use crate::backend::{Backend, CompileCtx, CompiledWhole, DispatchOutcome};
 use crate::ir::{Event, ProtoStatement};
 use std::sync::Arc;
 
+/// Turn chunk-local comb localization off for this process.
+///
+/// Localization keeps a comb signal in a C local and leaves its `comb_values`
+/// word holding whatever it last held.  Nothing outside the chunk reads it,
+/// so the simulation is unaffected and the validate dual-run skips those
+/// bytes — but a waveform dump shows them, and then disagrees with a run that
+/// took the Cranelift or interpreted path.  Waveform dumping is the caller.
+///
+/// Latches: once off, localization never comes back on.  Must be called
+/// before analysis, since the blocklist is computed during conv.
+pub fn force_disable_localize() {
+    emit::force_disable_localize();
+}
+
 pub struct AotCBackend {
     async_mode: bool,
     /// When false, only whole-comb compile is attempted.

crates/simulator/src/backend/aot_c/emit.rs

@@ -13,13 +13,14 @@ use crate::ir::{
     ProtoForBound, ProtoForRange, ProtoForStatement, ProtoStatement, ProtoSystemFunctionCall,
     VarOffset, native_bytes, veryl_aot_sysfn_print,
 };
+use crate::{HashMap, HashSet};
 use std::cell::RefCell;
-use std::collections::{HashMap, HashSet};
 use std::ffi::c_void;
 use std::fs;
 use std::io::ErrorKind;
 use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::mpsc::Sender;
 use std::sync::{Arc, Mutex, OnceLock};
 use std::thread;
@@ -270,22 +271,43 @@ fn reset_wide_tmp() {
 }
 
 // ---------------------------------------------------------------------------
-// Chunk-local comb intermediate localization (VERYL_AOT_C_LOCALIZE, default on,
-// `=0` to opt out).  A comb scalar written and read only within its emit chunk
-// is kept in a C local instead of round-tripping `comb_values` (gcc can't drop
-// the store — escaping restrict param — but the emitter's global read-set can).
+// Chunk-local comb intermediate localization (on by default;
+// `VERYL_AOT_C_LOCALIZE=0` or `force_disable_localize` opts out).  A comb
+// scalar written and read only within its emit chunk is kept in a C local
+// instead of round-tripping `comb_values` (gcc can't drop the store —
+// escaping restrict param — but the emitter's global read-set can).
 // Soundness: localize only a signal (a) written by one top-level unconditional
 // full-width scalar (≤64-bit) Assign, (b) read only in that chunk, (c) not
 // blocklisted (event-read / array-range / partial-write / port).  Blocklist
 // built in `module.rs`.
+
+/// Set by [`force_disable_localize`]; latches on and is never cleared, so
+/// localization can only ever be turned off, never back on.
+static LOCALIZE_FORCED_OFF: AtomicBool = AtomicBool::new(false);
+
+/// Whether chunk-local localization runs: on unless `VERYL_AOT_C_LOCALIZE=0`
+/// or a caller turned it off for the process.
+pub fn localize_enabled() -> bool {
+    if LOCALIZE_FORCED_OFF.load(Ordering::Relaxed) {
+        return false;
+    }
+    std::env::var("VERYL_AOT_C_LOCALIZE").as_deref() != Ok("0")
+}
+
+/// Latch the process-wide off switch; `super::force_disable_localize` is the
+/// public entry point and carries the rationale.
+pub fn force_disable_localize() {
+    LOCALIZE_FORCED_OFF.store(true, Ordering::Relaxed);
+}
+
 thread_local! {
     /// Comb offsets the caller marked unsafe to localize (read outside the
     /// comb function / dynamically / partial-written / port-visible).
     static LOCALIZE_BLOCKLIST: RefCell<HashSet<isize>> =
-        RefCell::new(HashSet::new());
+        RefCell::new(HashSet::default());
     /// Comb offsets localized in the chunk currently being emitted.
     static CURRENT_LOCAL: RefCell<HashSet<isize>> =
-        RefCell::new(HashSet::new());
+        RefCell::new(HashSet::default());
     /// Runtime-indexed comb array ranges (base, num_elements, stride) — a
     /// candidate offset inside any of these is excluded (a constant-indexed
     /// element could be read dynamically by an event / another statement).
@@ -687,7 +709,7 @@ fn compute_localize_sets(
             a.walk_stmt(s, i, true);
         }
     }
-    let mut sets: Vec<HashSet<isize>> = vec![HashSet::new(); chunks.len()];
+    let mut sets: Vec<HashSet<isize>> = vec![HashSet::default(); chunks.len()];
     for (off, wc) in &a.write_chunk {
         let Some(i) = wc else { continue };
         if a.bad.contains(off)
@@ -3389,7 +3411,7 @@ pub fn emit_function(stmts: &[ProtoStatement]) -> Option<String> {
         });
         sets
     } else {
-        vec![HashSet::new(); chunks.len()]
+        vec![HashSet::default(); chunks.len()]
     };
     clear_current_local();
     let mut chunk_bodies: Vec<String> = Vec::with_capacity(chunks.len());

crates/simulator/src/ir/module.rs

@@ -2910,16 +2910,15 @@ impl Conv<&air::Module> for ProtoModule {
             );
         }
 
-        // Chunk-local localization (VERYL_AOT_C_LOCALIZE): while events +
-        // derived-clock candidates are in scope, precompute the comb offsets the
-        // emitter must NOT localize — event-touched, in a runtime-indexed array
-        // range, or externally-visible (port / user-var / clock).
+        // Chunk-local localization (gated by `emit::localize_enabled`): while
+        // events + derived-clock candidates are in scope, precompute the comb
+        // offsets the emitter must NOT localize — event-touched, in a
+        // runtime-indexed array range, or externally-visible (port / user-var
+        // / clock).
         // LocalizeInfo = (blocklist offsets, array ranges).
-        type LocalizeInfo = (std::collections::HashSet<isize>, Vec<(isize, usize, isize)>);
+        type LocalizeInfo = (HashSet<isize>, Vec<(isize, usize, isize)>);
         #[cfg(not(target_family = "wasm"))]
-        let localize_info: Option<LocalizeInfo> = if std::env::var("VERYL_AOT_C_LOCALIZE")
-            .as_deref()
-            != Ok("0")
+        let localize_info: Option<LocalizeInfo> = if crate::backend::aot_c::emit::localize_enabled()
         {
             let event_slices: Vec<&[ProtoStatement]> = all_event_statements
                 .values()
@@ -2946,7 +2945,7 @@ impl Conv<&air::Module> for ProtoModule {
             for (_, off, _) in &nested_derived_clock_candidates {
                 block_vo.insert(*off);
             }
-            let mut block: std::collections::HashSet<isize> = std::collections::HashSet::new();
+            let mut block: HashSet<isize> = HashSet::default();
             for vo in &block_vo {
                 if !vo.is_ff() {
                     block.insert(vo.raw());
@@ -3256,7 +3255,7 @@ impl Conv<&air::Module> for ProtoModule {
             #[cfg(not(target_family = "wasm"))]
             if std::env::var("VERYL_AOT_C_DIAG").as_deref() == Ok("1") {
                 let census = crate::backend::aot_c::emit::comb_uncovered_census(&pre_jit_stmts);
-                let mut counts: std::collections::HashMap<String, usize> = Default::default();
+                let mut counts: HashMap<String, usize> = Default::default();
                 for c in census {
                     *counts.entry(c).or_default() += 1;
                 }

crates/veryl/src/cmd_test.rs

@@ -173,6 +173,14 @@ impl CmdTest {
     }
 
     pub fn exec(&self, metadata: &mut Metadata) -> Result<bool> {
+        // A dump wants every comb word, and localization leaves the ones no
+        // later reader needs holding stale values (see
+        // `aot_c::force_disable_localize`).  Before analysis: the blocklist is
+        // computed during conv.
+        if self.opt.wave {
+            veryl_simulator::backend::aot_c::force_disable_localize();
+        }
+
         // force filelist_type to absolute which can be refered from temporary directory
         metadata.build.filelist_type = FilelistType::Absolute;