Core: Make PropagateNaN min/max agree on signed-zero ties
Summary
pmin/pmax<PropagateNaN> did not select the same operand on a signed-zero tie as plain
pmin/pmax of the same type: the SSE/AVX/AVX512 helper passed op its operands reversed,
while PropagateFast and PropagateNumbers did not. The two rules meet inside every reduction,
so the sign of a zero result depended on where the vectorized body stopped and the scalar tail
began. Sign of v.maxCoeff<PropagateNaN>() for a vector of +0.0 with a single -0.0 last:
| n=1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | |
|---|---|---|---|---|---|---|---|---|---|
SSE4.2, float |
− | + | + | − | + | + | + | − | + |
SSE4.2, double |
− | − | + | − | + | − | + | − | + |
AVX2, float |
− | + | + | + | + | + | + | − | + |
AVX2, double |
− | + | + | − | + | + | + | − | + |
-0.0 appears exactly where n is a multiple of the packet size — the cases with no scalar
tail, where the packet rule wins outright. Identical under GCC 13 and Clang 18, so the
divergence is in Eigen rather than a compiler artifact.
The contract
pmin/pmax<PropagateNaN> may differ from plain pmin/pmax only where a NaN is
involved. Given two ordered operands both must select the same one, down to the sign of a zero
result. Signed zeros are the only operands that compare equal while differing in their bits, so
that is what this pins down.
Which operand a tie selects stays deliberately unspecified. It varies between backends, and between the scalar and packet forms of one backend, wherever the hardware min/max resolves a tie by sign rather than by position — see Follow-up.
The fix
The generic path passes op its operands in the caller's order, and SSE's
pminmax_propagate_nan tests b rather than a. op mirrors std::min/std::max and
already returns a when either operand is NaN, so testing b is exactly what supplies the
missing case, and it leaves op's operand order — hence its tie — intact.
NEON, RVV10, LSX and the generic clang backend already route PropagateNaN through their plain
op and are unchanged; the clang backend's pmin_nan_op is already written this way.
Every row of the table above becomes − + + + + + + + +, identically for both types under both
ISAs, and PropagateFast and PropagateNumbers now agree with it: maxCoeff() returns -0.0
only when the -0.0 is the first element.
Cost
One instruction per packet in a vectorized PropagateNaN reduction — testing the freshly loaded
operand stops it folding into vmaxpd's memory operand. Callgrind instruction counts, 200
reductions of 4096 doubles, -O3 -mavx2 -mfma:
| path | gcc 13 | clang 18 |
|---|---|---|
v.maxCoeff<PropagateNaN>() |
+4.63% | +4.67% |
v.cwiseAbs().maxCoeff<PropagateNaN>() |
0.00% | 0.00% |
c.real().cwiseAbs().maxCoeff<PropagateNaN>() |
0.00% (byte-identical) | 0.00% |
c.realView().cwiseAbs().maxCoeff<PropagateNaN>() |
0.00% | +7.17% |
Those percentages are relative to that harness's total. Re-measured since from a standalone
probe, the absolute figure is the one that travels: maxCoeff<PropagateNaN>() over 4096 doubles
costs +204,400 (gcc 13) and +204,600 (clang 18) instruction references across 204,800 packets —
0.998 and 0.999 instructions per packet — and exactly 0 once cwiseAbs() puts the operand in a
register.
bench_reductions registers these paths as of da2a3768, so the cost and any later recovery are
measurable rather than resting on the Callgrind recipe above: PropagateNaN and
PropagateNumbers rows for vector minCoeff/maxCoeff, and cwiseAbs(), real().cwiseAbs()
and realView().cwiseAbs() rows with and without PropagateNaN, over float, double,
complex<float> and complex<double>. PropagateNumbers is the control: it goes through the
sibling wrapper, which this change does not touch. The PropagateFast registrations and their
names are unchanged, since PropagateFast is the default template argument of
minCoeff()/maxCoeff().
Recovering it would mean swapping operand order in the shared Redux.h loops, for the scalar
and packet paths alike, which also moves every reduction's tie from the earliest to the latest
index and changes which NaN payload sum/prod propagate. That belongs in its own change.
Test
packetmath gained packetmath_minmax_propagation_test, which checks the invariant within
each type — the scalar form and every packet form — over signed zeros, signed infinities and
NaN. Zero results are compared bit-exactly, because isApprox cannot distinguish +0.0 from
-0.0; NaN payloads are not pinned across backends, so a NaN result only has to stay a NaN.
The check fails with either half of the fix reverted, verified independently for both.
It deliberately does not compare the scalar form against the packet form. That is what could not hold on NEON, and it is not what this change establishes.
PropagateNumbers is checked for its NaN semantics only: under EIGEN_VECTORIZE_GENERIC it is
reached through __builtin_elementwise_minnum, whose zero-sign tie is specified independently of
the plain compare-select form, so pinning it to plain pmin would assert something Eigen does
not claim.
The existing coverage could not catch any of this: the PropagateNaN coefficient-wise check fed
only {NaN, +0.0} — never -0.0 — and compared through isApprox.
Second commit: document realView()
realView() carried no documentation at all — DenseBase declared it without a Doxygen block
and no page mentioned it — so the one component view that can be linear and vectorizable was
undiscoverable from the strided views it can replace. It is now documented and added to the
\sa lists of real() and imag().
The claims are scoped to a complex *this, since for a real scalar both real() and
realView() return the source expression itself. For a complex *this, real() and imag()
build a CwiseUnaryView, which drops PacketAccessBit, so a reduction over either runs
coefficient by coefficient; RealView::Flags intersects the source flags, so linear access,
packet access and writability survive only where the source expression supplies them, and
writing additionally requires std::complex storage.
Validation
packetmath (15 splits) green in four configurations: GCC 13 at -msse4.2 and at
-mavx2 -mfma, Clang 18 at -mavx2 -mfma, and Clang 18 at -mavx2 -mfma -DEIGEN_VECTORIZE_GENERIC=1. array_cwise (41), redux (13), stable_norm (13) and
vectorization_logic (2) green in the first three — 207 further runs. reuse lint and
codespell clean, clang-format-17 applied, Doxygen reports nothing on the changed block.
The new bench_reductions registrations are verified to reach the changed helper by codegen, not
by timing: under -O3 -mavx2 -mfma every PropagateFast instantiation emits plain
vminpd/vmaxpd with no compare or blend, while every PropagateNaN and PropagateNumbers
instantiation emits vcmppd plus vblendvpd. They compile under GCC 13 at -msse4.2,
-mavx2 -mfma and -mavx512f -mavx512dq -mavx512vl -mfma, and under Clang 18 at -mavx2 -mfma
with and without EIGEN_VECTORIZE_GENERIC=1. They have not been timed: this host is under
unrelated load, and the Callgrind figures above remain the reproducible record until it is quiet.
Not run locally: aarch64 and riscv64, for want of a cross toolchain on this host. The argument
that they go green is by construction — their PropagateNaN specializations forward to their
plain op, so the invariant holds there without a code change — rather than by execution.
Follow-up
#3116 tracks the backends where the packet path resolves a tie by sign while the scalar path
resolves it by position, so a reduction's zero sign still depends on the packet width there:
NEON and RVV10, which reach pmin/pmax through vminq/vfmin, and GPU builds, where
numext::mini/maxi are specialized to number-preferring fmin/fmax. Recovering the
instruction above by reordering the min/max redux accumulation is a second follow-up.
Found while re-measuring #3097 (closed).