Commit d94f50b
committed
fix(codex-p1): VPABSB does NOT saturate — correct AVX-512 saturating_abs spec
Codex P1 on PR #400 caught that the canonical reference doc
(ndarray-vertical-simd-alien-magic.md §W1a #2) claimed
`_mm512_abs_epi8` saturates `i8::MIN → 127` by ISA. This is wrong:
VPABSB returns the same bit pattern for `0x80` (i.e., abs(i8::MIN)
= i8::MIN, since +128 doesn't fit in i8). A W1a worker implementing
the documented primitive would have shipped the same i8::MIN
divergence the spec was supposed to close.
Three files updated with the correct semantics:
1. `.claude/knowledge/ndarray-vertical-simd-alien-magic.md` §W1a #2
Correct AVX-512 impl: `_mm512_min_epu8(_mm512_abs_epi8(x),
_mm512_set1_epi8(0x7f))`. VPABSB gives the absolute-value bit
pattern; VPMINUB (unsigned min) then clamps the single
problematic byte 0x80 (=128 unsigned > 127) down to 0x7f
(=127). All other lanes are unchanged since `abs(x) < 0x80`
for `x ≠ i8::MIN`. NEON `vqabsq_s8` is already saturating
(the `q` suffix); scalar `i8::saturating_abs` is correct.
2. `.claude/board/EPIPHANIES.md` E-SIMD-SWEEP-1
Inline correction: `TD-NDARRAY-SIMD-SATURATING-ABS-I8` entry now
names the VPABSB+VPMINUB pair and explicitly notes that VPABSB
alone does NOT saturate i8::MIN.
3. `.claude/board/TECH_DEBT.md` TD-NDARRAY-SIMD-SATURATING-ABS-I8
Description rewrite: clarifies that PR #398's AVX-512 path got
the right answer not because of VPABSB but because it widens
i8 → i64 first and negate-blends (a different mechanism). The
new ndarray primitive must produce truly-saturating semantics
in the same byte-wide register without widening. Added a
mandatory test: `I8x16::saturating_abs(splat(i8::MIN))` must
return `splat(i8::MAX)` on all three backends.
Direction B verdict (scalar is buggy, AVX-512 outcome is correct)
is unchanged. The fix is to the IMPLEMENTATION STRATEGY for the
new ndarray primitive, not to the architectural decision.
Cross-ref: PR #400 codex P1 review; PR #398 codex P2 (the
i8::MIN divergence that motivated W1a-#2 in the first place);
Intel Intrinsics Guide for `_mm512_abs_epi8` (VPABSB);
ARM Architecture Reference for VQABS (`vqabsq_s8`).
https://claude.ai/code/session_01UwJuKqP828qyX1VkLgGJFS1 parent 460bfe5 commit d94f50b
3 files changed
Lines changed: 6 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
25 | | - | |
| 25 | + | |
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
35 | | - | |
| 35 | + | |
36 | 36 | | |
37 | | - | |
38 | | - | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
39 | 40 | | |
40 | 41 | | |
41 | 42 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
75 | 75 | | |
76 | 76 | | |
77 | 77 | | |
78 | | - | |
| 78 | + | |
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
| |||
0 commit comments