-
Notifications
You must be signed in to change notification settings - Fork 18
Description
Problem
In compiler/context.rs, IndexOp::ActiveDimRef was added to has_iteration_preserving_ops to support vector builtins (VectorSortOrder, VectorElmMap, etc.) where dimension references like h[DimA] must keep their full array view. However, the same preserve_wildcards_for_iteration flag is also set by array reducers (SUM, MIN, MAX, MEAN, SIZE, STDDEV).
This means if a vector builtin is nested inside a reducer (e.g., SUM(vector_sort_order(vals[*], 1)) + SUM(z[D])), the reducer's preserve_wildcards context could incorrectly convert ActiveDimRef to Wildcard for non-array arguments.
Why it matters
- Correctness: While not currently triggered, this is a latent semantic bug that could produce wrong simulation results in specific nesting patterns.
- Maintainability: The single flag conflates two distinct compiler intentions (reducer wildcard preservation vs. vector builtin dimension reference preservation), making future changes to either feature risky.
Component(s) affected
src/simlin-engine/src/compiler/context.rs--has_iteration_preserving_opsandpreserve_wildcards_for_iteration
Current state
Not triggered by any existing tests because:
- Normal lowering resolves dimensions before
preserve_wildcardsaffects them lower_preserving_dimensionsis only used for hoisting
Proposed fix
Introduce a separate context flag (e.g., promote_active_dim_ref) set only by vector builtins, keeping has_iteration_preserving_ops limited to Wildcard/SparseRange/Range. This cleanly separates the two concerns and prevents cross-contamination between reducer and vector builtin contexts.
Context
Identified during review of the VM vector operations branch (vm-vector-ops), specifically the addition of ActiveDimRef to iteration-preserving ops.