optimizer rework

CHANGELOG.md

@@ -7,28 +7,82 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- **Breaking:** operator attributes are keyed by the new `Attr` enum instead of
+  `String`. `OnnxNode.attributes` is now `FxHashMap<Attr, OnnxAttribute>` and the
+  `get_attr_*` helpers take an `Attr`. ONNX attribute names are a closed
+  vocabulary, but they were spelled as string literals at ~200 sites, where a
+  typo silently produced a missing attribute and a default value rather than a
+  compile error. Names outside the table live on in `Attr::Other`, so decoding
+  and re-exporting a model the runtime does not fully interpret stays lossless.
+
+- **Breaking:** `optimize_onnx_graph` now returns `Result<(), OnnxError>`.
+  Constant folding evaluates operators, so the pipeline can genuinely fail, and
+  the previous signature left no way to say so — failures were printed to stderr
+  and the model silently came back unoptimized. A node the evaluator declines is
+  still not an error: it is simply not folded.
+- The ONNX graph optimizer now runs over a def-use IR (`yscv-onnx/src/ir/`)
+  driven to a fixed point by a pass manager, instead of a hard-coded sequence of
+  functions mutating the model's string-keyed node list. Passes match through a
+  use list rather than positional adjacency, so producer/consumer pairs separated
+  by unrelated nodes now fuse. `YSCV_ONNX_PASSES=-name` disables an individual
+  pass; `YSCV_ONNX_PASS_LOG=1` reports each sweep.
+- Constant folding no longer builds a throwaway model and rebuilds the whole
+  execution plan per folded node; it dispatches the operator onto a bare
+  environment. It also folds a chain in one topological sweep rather than
+  rescanning from the start each time, refuses results that balloon far beyond
+  their inputs, and refuses the non-deterministic `Random*`/`Multinomial` family,
+  which would otherwise freeze a single draw into the weights.
+- `fuse_conv_relu`, `fuse_bn_relu`, `fold_conv_bn`, `fold_constants` and
+  `rewrite_convtranspose_dts` are no longer public. They were only ever reachable
+  through `optimize_onnx_graph` outside the crate.
+- Optimizer passes no longer rebuild the runtime index individually. Nine of
+  the twelve passes called `rebuild_runtime_index()` on exit, so a single
+  `optimize_onnx_graph` re-ran execution-plan construction and weight
+  prepacking about ten times. The driver now rebuilds once after the whole
+  pipeline; the public per-pass entry points still rebuild for standalone
+  callers.
+
 ### Fixed
 
+- Conv weight folding corrupted weights shared between two convolutions — the
+  shape a Siamese tracker's two branches produce. Each convolution folded its own
+  BatchNormalization into the shared tensor, applying the second scale on top of
+  the first. Sharing is now detected and the fold declines.
+- `Conv` + `Mul`/`Add` folding treated any constant with one element per output
+  channel as per-channel, including a rank-1 `[OC]` tensor. ONNX broadcasting
+  aligns trailing axes, so against an NCHW convolution output that scales *width*,
+  not channels. Only a scalar, or a constant whose channel-aligned axis holds the
+  output-channel count with every other axis 1, now qualifies.
+- Synthesized bias operands were named from the node name, which ONNX makes
+  optional, so every unnamed convolution proposed the same name and the second
+  silently aliased the first's tensor.
 - `remove_dropout_nodes` deleted nodes by `NodeProto.name`, which ONNX makes
   optional. A single unnamed `Dropout` put the empty string into the delete set
   and took every other unnamed node in the graph with it — on a fully unnamed
   three-node fixture the pass emptied the graph. It also deleted malformed
   `Dropout` nodes it had skipped rewiring. Now removes only the nodes it
   actually rewired, by index.
+- `Transpose` + `MatMul` fusion fired even when the transposed value had a
+  consumer that could not absorb it. The fused action reads the value *before*
+  the transpose, so when another consumer forced the `Transpose` to run anyway,
+  running it consumed that value and the fusion then read a tensor that was
+  gone — a `Transpose` feeding both a `MatMul` and any other op failed the
+  inference outright. The matcher now requires every reader of the transposed
+  value to be an absorbing `MatMul`, which is what its own comment always
+  claimed.
+- The `PW_expand → DW → PW_reduce` merge matched its residual `Add`
+  positionally while every other matcher worked by dataflow. When the two
+  disagreed the merge absorbed the pointwise reduction without its `Add`, and
+  the addition was dropped from the execution plan entirely — inference
+  returned successfully with a graph output missing. It now takes over whatever
+  the `Conv`+`Add` matcher resolved, or declines.
 - `eliminate_squeeze_unsqueeze_pairs` could accept overlapping inverse pairs
   `(i, i+1)` and `(i+1, i+2)` from a chain such as `Squeeze -> Unsqueeze ->
   Squeeze`. The reverse-removal loop then deleted already-shifted indices,
   taking unrelated nodes with them. Overlapping matches are now skipped.
 
-### Changed
-
-- Optimizer passes no longer rebuild the runtime index individually. Nine of
-  the twelve passes called `rebuild_runtime_index()` on exit, so a single
-  `optimize_onnx_graph` re-ran execution-plan construction and weight
-  prepacking about ten times. The driver now rebuilds once after the whole
-  pipeline; the public per-pass entry points still rebuild for standalone
-  callers.
-
 ### Removed
 
 - `run_onnx_model_sequential`, the ~600-line per-inference fusion scanner in

apps/llm-bench/src/bin/bench_tracker.rs

@@ -5,8 +5,8 @@ use std::process::ExitCode;
 use std::time::Instant;
 
 use yscv_onnx::{
-    OnnxAttribute, OnnxModel, OnnxNode, OnnxRunner, dump_runner_profile, load_onnx_model_from_file,
-    optimize_onnx_graph, quant_runtime_stats, reset_quant_runtime_stats,
+    Attr, OnnxAttribute, OnnxModel, OnnxNode, OnnxRunner, dump_runner_profile,
+    load_onnx_model_from_file, optimize_onnx_graph, quant_runtime_stats, reset_quant_runtime_stats,
 };
 use yscv_tensor::Tensor;
 
@@ -90,8 +90,8 @@ fn tensor_len(shape: &[usize]) -> usize {
     shape.iter().product()
 }
 
-fn attr_int(node: &OnnxNode, name: &str, default: i64) -> i64 {
-    match node.attributes.get(name) {
+fn attr_int(node: &OnnxNode, name: Attr, default: i64) -> i64 {
+    match node.attributes.get(&name) {
         Some(OnnxAttribute::Int(v)) => *v,
         _ => default,
     }
@@ -109,7 +109,7 @@ fn qlinear_conv_kind(model: &OnnxModel, node: &OnnxNode) -> Option<&'static str>
     if shape.len() != 4 {
         return None;
     }
-    let group = attr_int(node, "group", 1) as usize;
+    let group = attr_int(node, Attr::Group, 1) as usize;
     if group == 1 && shape[2] == 1 && shape[3] == 1 {
         return Some("pw");
     }
@@ -203,7 +203,7 @@ fn quant_chain_candidates(model: &OnnxModel) -> usize {
 fn run(args: Args) -> Result<(), String> {
     let mut model =
         load_onnx_model_from_file(Path::new(&args.model)).map_err(|e| format!("load: {e}"))?;
-    optimize_onnx_graph(&mut model);
+    optimize_onnx_graph(&mut model).map_err(|e| format!("optimize: {e}"))?;
     let chain_candidates = quant_chain_candidates(&model);
     let runner =
         OnnxRunner::with_threads(&model, args.threads).map_err(|e| format!("runner: {e}"))?;

apps/llm-bench/src/bin/calib_accuracy.rs

@@ -212,7 +212,7 @@ fn collect_real_activations(
     // exec paths rely on. Skipping this step makes Conv kernels see
     // raw OIHW shapes and triggers "bias shape mismatch" on the first
     // conv layer.
-    optimize_onnx_graph(&mut model);
+    optimize_onnx_graph(&mut model).map_err(|e| format!("optimize: {e}"))?;
     let runner = OnnxRunner::new(&model).map_err(|e| format!("runner: {e}"))?;
     let coll = CalibrationCollector::new();
     coll.enable_histograms(true);

apps/llm-bench/src/bin/nchwc_coverage.rs

@@ -9,7 +9,7 @@ use rustc_hash::FxHashMap;
 ///   cargo run --release --no-default-features -p yscv-llm-bench --bin nchwc_coverage \
 ///     -- private/private/model.onnx
 use rustc_hash::FxHashSet;
-use yscv_onnx::{OnnxAttribute, OnnxModel, OnnxNode, load_onnx_model_from_file};
+use yscv_onnx::{Attr, OnnxAttribute, OnnxModel, OnnxNode, load_onnx_model_from_file};
 
 fn main() {
     let args: Vec<String> = std::env::args().collect();
@@ -18,16 +18,16 @@ fn main() {
     run_coverage_probe(&model);
 }
 
-fn get_int_attr(node: &OnnxNode, key: &str, default: i64) -> i64 {
-    match node.attributes.get(key) {
+fn get_int_attr(node: &OnnxNode, key: Attr, default: i64) -> i64 {
+    match node.attributes.get(&key) {
         Some(OnnxAttribute::Int(v)) => *v,
         Some(OnnxAttribute::Ints(v)) => v.first().copied().unwrap_or(default),
         _ => default,
     }
 }
 
-fn get_ints_attr(node: &OnnxNode, key: &str) -> Vec<i64> {
-    match node.attributes.get(key) {
+fn get_ints_attr(node: &OnnxNode, key: Attr) -> Vec<i64> {
+    match node.attributes.get(&key) {
         Some(OnnxAttribute::Ints(v)) => v.clone(),
         _ => vec![],
     }
@@ -150,11 +150,11 @@ fn run_coverage_probe(model: &OnnxModel) {
             .cloned()
             .unwrap_or_default();
 
-        let group = get_int_attr(node, "group", 1);
-        let strides = get_ints_attr(node, "strides");
+        let group = get_int_attr(node, Attr::Group, 1);
+        let strides = get_ints_attr(node, Attr::Strides);
         let sh = strides.first().copied().unwrap_or(1);
         let sw = strides.get(1).copied().unwrap_or(1);
-        let dilations = get_ints_attr(node, "dilations");
+        let dilations = get_ints_attr(node, Attr::Dilations);
         let dh = dilations.first().copied().unwrap_or(1);
         let dw_d = dilations.get(1).copied().unwrap_or(1);
 
@@ -384,11 +384,11 @@ fn run_coverage_probe(model: &OnnxModel) {
                     .cloned()
                     .unwrap_or_default();
 
-                let group = get_int_attr(node, "group", 1);
-                let strides = get_ints_attr(node, "strides");
+                let group = get_int_attr(node, Attr::Group, 1);
+                let strides = get_ints_attr(node, Attr::Strides);
                 let sh = strides.first().copied().unwrap_or(1);
                 let sw = strides.get(1).copied().unwrap_or(1);
-                let dilations = get_ints_attr(node, "dilations");
+                let dilations = get_ints_attr(node, Attr::Dilations);
                 let dh = dilations.first().copied().unwrap_or(1);
                 let dw_d = dilations.get(1).copied().unwrap_or(1);
 

apps/llm-bench/src/bin/quantize_tracker.rs

@@ -444,7 +444,7 @@ fn run(args: Args) -> Result<(), String> {
     eprintln!("loading {}…", args.model);
     let mut model_fp32 =
         load_onnx_model_from_file(Path::new(&args.model)).map_err(|e| format!("load fp32: {e}"))?;
-    optimize_onnx_graph(&mut model_fp32);
+    optimize_onnx_graph(&mut model_fp32).map_err(|e| format!("optimize fp32: {e}"))?;
     let runner_fp32 = OnnxRunner::new(&model_fp32).map_err(|e| format!("runner fp32: {e}"))?;
 
     let calibration_samples = if let Some(spec) = args.calibration_jsonl.as_ref() {
@@ -509,7 +509,7 @@ fn run(args: Args) -> Result<(), String> {
     );
     let mut model_qdq =
         load_onnx_model_from_file(Path::new(&args.model)).map_err(|e| format!("load q: {e}"))?;
-    optimize_onnx_graph(&mut model_qdq);
+    optimize_onnx_graph(&mut model_qdq).map_err(|e| format!("optimize q: {e}"))?;
     match args.format {
         QuantFormat::Qdq => rewrite_to_qdq(&mut model_qdq, &stats, &args.keep_fp32)
             .map_err(|e| format!("rewrite_to_qdq: {e}"))?,
@@ -556,7 +556,7 @@ fn run(args: Args) -> Result<(), String> {
         let mut reloaded = load_onnx_model_from_file(Path::new(output_path))
             .map_err(|e| format!("reload: {e}"))?;
         if args.format == QuantFormat::Qdq {
-            optimize_onnx_graph(&mut reloaded);
+            optimize_onnx_graph(&mut reloaded).map_err(|e| format!("optimize reload: {e}"))?;
         }
         model_qdq = reloaded;
     }

benchmarks/onnx-models/benches/onnx_models.rs

@@ -18,7 +18,8 @@ fn bench_onnx_models(criterion: &mut Criterion) {
         .map(|case| {
             let mut model = load_onnx_model_from_file(asset_dir.join(&case.model))
                 .unwrap_or_else(|error| panic!("load {}: {error}", case.model));
-            optimize_onnx_graph(&mut model);
+            optimize_onnx_graph(&mut model)
+                .unwrap_or_else(|error| panic!("optimize {}: {error}", case.model));
             let inputs = make_inputs(&case, &asset_dir).expect("build model benchmark inputs");
             PreparedCase {
                 name: case.name,

benchmarks/onnx-models/src/main.rs

@@ -576,7 +576,7 @@ fn run_case(
     let mut model = load_onnx_model_from_file(model_path)
         .map_err(|e| format!("load {}: {e}", model_path.display()))?;
     let nodes_before = model.node_count();
-    optimize_onnx_graph(&mut model);
+    optimize_onnx_graph(&mut model).map_err(|e| format!("optimize: {e}"))?;
     let nodes_after = model.node_count();
     let runner = if threads == 0 {
         OnnxRunner::new(&model).map_err(|e| format!("runner init: {e}"))?

crates/yscv-onnx/README.md

@@ -27,7 +27,7 @@ let outputs = runner.run(&[("images", &input)])?;
   - `run_onnx_model_borrowed` (borrowed `FxHashMap`)
   - `run_onnx_model_borrowed_slice` (borrowed `&[(&str, &Tensor)]`, no `FxHashMap` required)
 - **122 ONNX CPU operators**: Conv, MatMul, Gemm, Relu/LeakyRelu/Sigmoid/Tanh/Gelu/Erf/Mish/HardSwish/Softmax/LogSoftmax, BatchNormalization/LayerNormalization/InstanceNormalization, MaxPool/AveragePool/GlobalAveragePool, Resize/Upsample, Concat/Split/Reshape/Flatten/Transpose/Gather/GatherElements/GatherND/ScatterElements/ScatterND/Slice/Tile/Expand, Cast/Pad/Clip/Where/Identity/CumSum/ArgMax/ArgMin/TopK, DepthToSpace/SpaceToDepth, GridSample/RoiAlign/NonMaxSuppression, full quantized stack (QuantizeLinear/DequantizeLinear/QLinearConv/QLinearMatMul/MatMulInteger/ConvInteger/DynamicQuantizeLinear), trig + hyperbolic, logical, fused `Conv_Relu` / `BatchNormalization_Relu`, … (see the op dispatch in `src/runner/dispatch.rs`)
-- **Graph optimizations**: constant folding, Conv+BN fusion, Conv+Relu fusion, dead node elimination
+- **Graph optimizations**: eleven passes over a def-use IR (`src/ir/`), run to a fixed point by a pass driver — Dropout removal, inverse Squeeze/Unsqueeze elimination, ConvTranspose(k==s) → Conv1x1 + DepthToSpace, Conv+BatchNormalization folding, Conv+Mul / Conv+Add constant absorption, constant folding, Conv+Relu and BN+Relu fusion, dead node elimination, and a topological reorder. Individual passes can be disabled with `YSCV_ONNX_PASSES=-name`; `YSCV_ONNX_PASS_LOG=1` reports what each sweep changed.
 - **Runtime fusion**: Conv+SiLU (Conv→Sigmoid→Mul pattern), Conv+Relu, BN+Relu, Gemm+Relu, Add+Relu (in-place with buffer reuse), Conv+Add residual (in-place, buffer reuse), in-place Add (same-shape last-use)
 - **NHWC layout**: Conv outputs stored in NHWC for cache-friendly depthwise/pointwise chains; per-slot layout tracking with automatic NCHW permute when needed. Conv+Add fusion captures NHWC flag before `env.remove()` to prevent layout corruption in residual chains.
 - **Quantized Conv export**: QDQ/QLinear rewrites serialize Conv weights in standard OIHW form even when the loader had normalised them to internal KHWC, grouped-KHWC, or depthwise-KHWC layouts; yscv restores the fast NHWC layouts on reload while exported models stay ONNX/ORT-compatible.