feat(standalone): emit FP32/rv32imf platform kernels for Stage-2 prob…

[FeelTheBeats]

…lems

The platform's Stage-2 paper (fwht/winograd/spmm) is float32 with -march=rv32imf and self-describing inputs; the previous Q16.16 int32 kernels could never pass it.

• _emit_platform_fwht: int32 -> float32 (flw/fsw/fadd.s/fsub.s), same butterfly order as the oracle

• _emit_platform_conv: rewrite to the winograd ABI ([batch,H,W,Cin,Cout,K] header, NHWC feat, OIHW weights, NCHW out, SAME pad, no bias)

• _emit_platform_spmm: rewrite to the spmm ABI ([M,K,N,nnz] + row_ptr + col_idx int32 + values + B float32)

• emit_platform_asm: route Conv/WinogradConv to the FP32 conv kernel

• tests: extend the in-test interpreter with RV32F ops; move platform-kernel and Stage-2 references to float32/new ABI

Verified: platform_asm + kernels 16 passed; Stage-2 30 passed; submitted to the platform -> fwht 30/30, winograd 30/30, spmm 40/40.

[github-actions]

🤖 AI Code Review

共审查 2 个变更文件

📁 scratchv/standalone/onnx_to_riscv_standalone.py

Code Review: scratchv/standalone/onnx_to_riscv_standalone.py

🔴 ABI break is uncoordinated — Conv and SpMM headers changed shape, layouts flipped, and a2 is now ignored.
The Conv kernel no longer reads the param block from a2; it reinterprets a0 as [batch,H,W,Cin,Cout,K] and expects NHWC feature data (old code read NCHW via ((b*Cin+ic)*H+ih)*W+iw). SpMM likewise moved from a0=pointers, a2=params to a self-describing a0. Nothing in this diff shows the generator emitting the new headers or NHWC layout. If the caller still emits the old NCHW feature map + param block, the kernel reads garbage silently. Verify the emitting side is updated in lockstep, and consider an assert on header shape at emit time so a mismatch fails loudly instead of producing wrong numbers.

🟡 Conv bias silently dropped — Old kernel loaded bias[oc] when b_ptr != 0 (_p_conv_nobias). New kernel hardcodes fmv.w.x ft0, zero and the header has no b_ptr field.
If riscv_oracle._winograd / the platform reference folds a bias in, every output cell is off by the bias value — well outside rtol=1e-4. Confirm the baseline is genuinely bias-free; otherwise add a b_ptr/bias[ Cout] section and restore the init.

🟡 FP32 accumulation order is load-bearing — Conv accumulates c → kh → kw; SpMM accumulates over p. Both docstrings assert bit-for-bit match with the oracle, which is only true if the oracle uses the identical order (FP32 is non-associative).
The claim is plausible but should be stated as an invariant in the loop comments (not just the docstring), so a future "optimization" that reorders these loops doesn't break the tolerance. Consider # ORDER-SENSITIVE: do not re-nest.

💭 All 12 s-registers consumed in both Conv and SpMM — Conv uses s0–s11; SpMM uses s0–s11. Both are leaf functions (no call), so t-registers are free and safe. Using s11 (x27) and the rest leaves zero headroom for debugging or a future inner helper. Consider moving loop counters (s9 batch, s10 oc, s11 oh / s6 i, s9 j, s10 p) to t-registers.

💭 pad = K/2 is baked in — srli s6, s5, 1 with no pad in the header. Correct for SAME with K∈{3,5}, but silently wrong for any other config. The docstring documents it, so this is fine — just noting it's a hard constraint on which ONNX nodes this path is valid for. Worth a guard in emit_platform_asm (or its caller) rejecting even K or non-SAME conv rather than emitting a wrong kernel.

💭 SpMM layout arithmetic is correct but fragile — s2=a0+16 (row_ptr), s3=s2+(M+1)*4 (col_idx), s4=s3+nnz*4 (values), s5=s4+nnz*4 (B). Verified each offset and the B[k*N+j]/C[i*N+j] indexing checks out. Since correctness depends on the header being exactly 4 int32 followed by row_ptr, the fragile part is the constant 16; a named comment (# header = 4 × int32 = 16 bytes) would help the next reader.

---

📁 tests/test_platform_kernels.py

🔴 Bug: double-rounding in reference may diverge from kernel — SPMM & Conv references

# SPMM reference (line ~186)
acc = _f32(acc + _f32(values[p] * float(B[col[p]][j])))

values[p] * float(...) computes in Python double-precision, then _f32 rounds to float32. The actual kernel multiplies two float32 registers directly (single rounding). Double-rounding differs from single-rounding in edge cases (e.g., when the true product's ULP boundary falls between two float32 values).

Suggestion: Compute products in numpy float32 to match hardware semantics:

import numpy as np
acc = np.float32(acc + np.float32(np.float32(values[p]) * np.float32(B[col[p]][j])))

Same issue in the conv reference (_f32(x * wv)).

---

🟡 Conv output layout undocumented — Line ~172

Input is NHWC (batch, H, W, Cin) but output is read as NCHW (batch, Cout, Hout, Wout). This mismatch works but will confuse future readers.

Suggestion: Add a comment: # Output is NCHW regardless of NHWC input.

---

🟡 Conv reference doesn't apply bias — Original kernel had a bias array; the new test omits it. If the kernel still adds bias internally, the reference is wrong.

Suggestion: Confirm the kernel was updated to remove bias, or add bias back to both kernel and reference.

---

🟡 FP register lambdas defined inside loop — Line ~67

gf and sf are recreated every instruction. Harmless for correctness, but the existing g/st lambdas have the same pattern — consider hoisting all four outside the while for consistency and clarity.

---

💭 _f32 on already-f32 values** — Several reference call sites apply _f32 to values that are already float32 (e.g., _f32(x + v) where x and v come from a list initialized with _f32 results). Redundant but harmless.

💭 batch is always 1 — Conv test introduces a batch variable that's hardcoded to 1. Consider adding it to @pytest.mark.parametrize later if batch > 1 support is added to the kernel.

💭 flw/fsw parsing requires explicit 0(reg) — int("") will raise ValueError for zero-offset syntax like flw ft0, (t0). Consistent with existing lw/sw, but worth a defensive fallback if kernels may emit offset-less forms:

off = int(r[1][:r[1].index("(")]) if r[1][:r[1].index("(")] else 0

---