Cycle 5 closed: CDEF QPU R5=0.116 ORANGE, opportunistic helper

Phase 4 plan with 3 Phase-5 REDs applied inline:
  - meta layout: m.z=tmp_off, m.w=dir
  - sec_shift clamped to >=0 (NEON uqsub semantics)
  - directions table as const ivec2[14], not OR-packed

Phase 6 deliverable: v3d_cdef.comp (387 inst, 2 threads, no spills).
3-way M1 (QPU vs C ref vs NEON) PASS 4096/4096.

M2: 0.443 Mblock/s -> R5 = 0.116 ORANGE (predicted 0.02-0.05 RED).
M4 same-kernel: NEON-3+QPU 8.46 < NEON-4 alone ~10 (negative).
M4 mixed (NEON-3 MC + QPU CDEF): CPU 34.17 Mblock/s MC,
  QPU 0.42 Mblock/s CDEF helper. CPU side higher than the
  Issue 003 NEON-fallback proxy suggested - cross-substrate
  contention is gentler than same-side NEON contention.

Verdict: CDEF stays on CPU; QPU dispatch path exists for
opportunistic use. Deployment recipe table updated for all 5
cycles. Phase 9 lessons: linear extrapolation across cycles is
too pessimistic; CDEF is bandwidth-bound on NEON despite high
per-block ns; real-substrate-cross contention < NEON-proxy
contention.

- src/v3d_cdef.comp: cycle 5 QPU shader
- tests/bench_v3d_cdef.c: 3-way M1, M2 bench
- tests/bench_concurrent_mixed.c: K_CDEF on both sides
- tests/cdef_ref.c + bench_neon_cdef.c: sec_shift clamp +
  expanded damping range to exercise the edge case
- CMakeLists.txt: v3d_cdef.spv + bench_v3d_cdef wiring
- docs/k5_cdef_phase4.md updated with Phase 5 review applied
- docs/k5_cdef_phase7.md: closure doc with full verdict matrix

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

docs/k5_cdef_phase4.md

@@ -56,17 +56,18 @@ Output: `dst[r,c] = clamp(px + ((sum - (sum<0) + 8) >> 4), min, max);`
 - **No shaderFloat16/Int8 ALU**: int math everywhere. uint8 dst
   via storageBuffer8BitAccess (cycle-1 v4 pattern).
 
-## SSBO layout
+## SSBO layout (post Phase 5 RED-1 fix)
 
-- `Meta[i]`: `uvec4(dst_off_bytes, params0, params1, dir)` where
-  `params0 = (pri | sec << 8 | damping << 16)` and
-  `params1 = tmp_off_bytes` (offset to block-origin = padded_origin + 2*16+2)
-- `Tmp[]`: `uint16` array (`uint8_t` SSBO with manual 16-bit
-  read? Or `storageBuffer16BitAccess`? V3D 7.1 supports the
-  16-bit extension.)
-- `Dst[]`: `uint8_t` array
-
-Use 16-bit storage extension for tmp.
+- `Meta[i]`: `uvec4(dst_off_bytes, params0, tmp_off_u16, dir)` —
+  i.e. `m.x` = dst_off, `m.y` = params (pri | sec << 8 |
+  damping << 16), `m.z` = tmp block-origin u16-element offset,
+  `m.w` = dir (3 bits used). **Pseudo-code below uses this
+  layout consistently.**
+- `Tmp[]`: `uint16_t` array via `GL_EXT_shader_16bit_storage` +
+  `storageBuffer16BitAccess` — both already enabled in
+  `v3d_runner.c` and used by cycle 1 IDCT shader. No uncertainty.
+- `Dst[]`: `uint8_t` array via `GL_EXT_shader_8bit_storage` (per
+  cycle-1 v4 pattern).
 
 ## Lane decomposition
 
@@ -89,14 +90,20 @@ layout(push_constant) uniform PC {
 } pc;
 ```
 
-## Directions table
+## Directions table (post Phase 5 RED-3 fix)
 
-Store the 14-entry stride-16 directions table as a `const uint
-dirs[14]` in the shader, packed as `(off1 << 16) | off2` per
-direction (both signed offsets fit in int16). Read via index.
+Use `const ivec2 dirs[14]` (8 directions + 6 wrap copies), each
+entry = `(off_k0, off_k1)`. Signed-int storage handles negative
+offsets cleanly without manual sign-extension. The OR-pack
+approach proposed earlier would corrupt negative offsets;
+abandoned.
 
-Alternative: store as constants array (compiler may unroll into
-uniform LUT). Same as cycle-2 LPF stored its tap weights.
+Values from `tests/cdef_ref.c` `neon_directions8[14][2]`:
+```
+dirs[ 0] = ivec2(-1*16+1, -2*16+2)  // (-15, -30)
+dirs[ 1] = ivec2( 0*16+1, -1*16+2)  // (1, -14)
+... (etc.)
+```
 
 ## Shader pseudo-code
 
@@ -114,18 +121,18 @@ void main() {
 
     uvec4 m = u_meta.meta[block_idx];
     uint dst_off = m.x + row * pc.dst_stride_u8 + col;
-    uint tmp_off = m.w + row * pc.tmp_stride_u16 + col;   // m.w = tmp block-origin u16 offset
+    uint tmp_off = m.z + row * pc.tmp_stride_u16 + col;   // m.z = tmp block-origin u16 offset
     int pri = int(m.y & 0xffu);
     int sec = int((m.y >> 8) & 0xffu);
     int damping = int((m.y >> 16) & 0xffu);
-    int dir = int(m.z & 7u);
+    int dir = int(m.w & 7u);
 
     int px = int(u_tmp.tmp[tmp_off]);
     int sum = 0;
     int mn = px, mx = px;
 
     int pri_shift = max(0, damping - ulog2(pri));
-    int sec_shift = damping - ulog2(sec);
+    int sec_shift = max(0, damping - ulog2(sec));  // RED-2: NEON uqsub saturates to 0; GLSL >> by negative is UB.
 
     // pri_tap[k] for k=0,1 = 4-(pri&1), then (tap & 3) | 2
     int pri_tap0 = 4 - (pri & 1);
@@ -174,6 +181,25 @@ shaderdb prediction:
 - uniform count: 14 entries × 2 offsets = 28; + tap weights 4
   = small. Should stay well below threshold. Predict 4 threads.
 
+## Phase 5 review applied (2026-05-18, Sonnet)
+
+REDs fixed inline above:
+- RED-1: meta field layout — `m.z = tmp_off`, `m.w = dir` (was swapped).
+- RED-2: `sec_shift = max(0, ...)` to match NEON's `uqsub` saturation.
+- RED-3: directions table is `const ivec2 dirs[14]`, not packed.
+
+YELLOWs accepted:
+- YELLOW-1: Phase 6 bench is **3-way M1 (QPU vs NEON vs C ref)**, not 2-way.
+- YELLOW-2: 16-bit storage extension confirmed present (cycle-1 already uses it).
+- YELLOW-3: `sec_tap0 = 2, sec_tap1 = 1` made explicit in shader.
+- YELLOW-4: use `gl_WorkGroupID.x` directly, not `gid / 256u`.
+
+**Also**: also clamp `sec_shift` in `tests/cdef_ref.c` (currently
+unguarded; M1 gate passes by bench-param luck — params don't
+exercise negative shift). Fix C ref + add negative-shift cases to
+bench param generator so the 3-way M1 actually stresses the
+edge case.
+
 ## Phase 5 review focus
 
 Particular review items for the Phase 5 second-model audit: