[Noah-Fotenos]

Added vfrec7 module.

Claude review: Fix illegal nested always_comb (would not compile) found during review.

Claude-Session: https://claude.ai/code/session_01WwBGrU45A1ohBuBdbTnx2m

[davidharrishmc]

Please click "Resolve conversation" if you are confident your changes resolve the query. Otherwise, reply with a note.

[davidharrishmc]

Please ask for an AI review of your own work. Don't assume everything it says is true. Here is what Claude says. Q is not applicable. The code has significant unnecessary hardware and could be simplified to a single short module.

PR #1858 — vfrec7 vector reciprocal estimate unit

2026-09-24 — critical review at head 89a5b12ca

Target: #1858, Noah-Fotenos:Vfpu → vector (base e920dc942,
merge base bdeffc504), 2 commits. David requested changes at b68f30f98 (7 inline comments, 2026-09-13);
89a5b12ca (2026-09-20) splits the module into 7 files, adds a $readmemh ROM and a Verilator unit TB.
Not instantiated anywhere; no CI checks run on the vector base.

Harness (durable copies in pr1858-vfrec7/): ref.py = spec-derived Python model (V spec 13.10 text + table
parsed from riscv-isa-manual vfrec7.edn; reproduces the spec's two worked examples); tb_x.sv = unpackinput →
vfrec7, reads vec.txt, writes out.txt. python3 ref.py gen then build with Verilator (rv64gc include dirs,
cvw.sv, unpackinput.sv, lzc.sv, vfrec7/*.sv, the PR's wrapper.c), run with WALLY set, python3 ref.py cmp.

Verdict: functionally correct; hardware and test bench need another round

Verified:

• ROM = spec table exactly (all 128 entries).
• 1,143,936 vectors, 0 mismatches: exhaustive FP16 × RNE/RTZ/RDN/RUP/RMM; FP32 every exponent × every LUT
  index × 3 low-bit patterns × sign; FP64 exponents near 0/bias/max; FP32/FP64 subnormals at every
  leading-zero count × every 7-bit window × 5 rm; 200k random per width. Harness catches a single ROM entry
  off by 2 (8106 mismatches), so it has teeth.
• Lint (lint-wally flags, vfrec7 as top, rv64gc/rv32gc): only UNUSEDPARAM RNE/RMM in vfrec7special.sv:55,59.

Findings, ranked:

1. LZC + 52-bit shifter are unnecessary (vfrec7norm.sv:42-56). Only 0 or 1 leading zeros reach the table
   (≥2 overflows), so Xm[NF-1], Xm[NF-2] and a 3:1 mux of 7-bit slices replace them. Direct answer to
   David's "share the three LZCs" comment: none needed. Synthesis cannot prune them (the top 7 bits of a variable
   left shift depend on every input bit).
2. Bigger, not smaller: 254 code lines across 7 modules vs David's "under 100 lines". Compact rewrite
   pr1858-vfrec7/vfrec7-compact.sv = 63 lines total, same interface, passes all 1.14M vectors, lint-clean.
   Tricks: max finite = infinity − 1 in every format; derive per-SEW constants from one 13+52-bit value sliced
   per SEW instead of four constant tables in vfrec7fmt.
3. $readmemh ROM is not synthesizable in synthDC (vfrec7lut.sv:38-47): DC ignores initial blocks and
   synth.tcl copies only *.sv into hdl/. Works for Verilator/Questa/Vivado. Precedent in src/ is only the
   FPGA-preload memories. A localparam logic [6:0] RECIP7 [0:127] = '{...} is as compact and works everywhere,
   no DPI/$WALLY. (David's own suggestion introduced the $readmemh; flag gently.)
4. TB exit status is always 0 (tb_vfrec7.sv:347): $finish(1) is a diagnostic level, not an exit code.
   Confirmed with a corrupted ROM: FAILURES: 44/6368 and exit 0.
5. TB can't catch table errors: checks |x·y−1| ≤ 2^-6 though vfrec7 is bit-exact in the spec (the comment
   "the spec guarantees 2^-7" is wrong). One entry 0x6e→0x6c passes ALL PASS. Subnormal sweep
   (:303) puts idx>>(k+1) in the low 6 bits, so SEW=32/64 always hit LutIdx 0. Output exponent 0
   (DenormShift=1) never exercised (exps tested: 1, B+1, 2B). Cross-SEW check (:318-340) is tautological
   (same LUT by construction). Fix: compare exactly against a spec model (e.g. Spike's f*_recip7 or port
   ref.py).
6. Nothing enforces the "D and Zfh, no Q" assumption (vfrec7.sv:45): 13'(P.D_BIAS) as the unpacked bias is
   wrong when Q is supported, and {(P.FLEN-64){1'b0}} is a negative replication when FLEN=32. Add an
   elaboration $error or generate guard. vfma has the same assumption as a TODO.
7. Interface differs from sibling vfma (packed operands in, unpacks internally); vfrec7 takes unpacked fields.
   Ask which the VPU lane will provide.
8. Housekeeping: ValidSew/SEW=8 path is dead if the decoder rejects SEW=8; author email nfotneos (all 7
   headers); TB header names src/vpu/vfrec7.sv; unit TB in tests/custom/ (RISC-V program dir) and not in any
   regression; wrapper.c near-duplicate of sim/verilator/wrapper.c; commit 1 message body describes a fix,
   commit 2 "Adressing"; PR body is one line.

Not posted to GitHub.

[davidharrishmc]

Overall, this looks clean. However, Claude offers a design 12% smaller and 7.5% faster. Do you want to spend some time optimizing yourself, or see Claude's design?

[Noah-Fotenos]

Change to ZVFH once supported in Wally. Currently, ZFH is used to test synthesis size effects.

[davidharrishmc]

Just add Zvfh_support in the Wally config. Follow it through the P. to get it to modules. You’ll probably have to do this for a bunch of other parameters.

…

On Thu, Oct 22, 2026 at 10:30 AM Noah-Fotenos ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/vpu/vfrecrsqrt7.sv <#1858 (comment)> : > @@ -89,28 +96,40 @@ module vfrecrsqrt7 import cvw::*; #(parameter cvw_t P) ( // internal biases are both odd, so that LSB is Xe[0] for a normal and Z[0] for a subnormal. assign LutOut = IsSqrt ? REC7SQRT_TBL[{XSubnorm ? ZeroCount[0] : Xe[0], Idx[6:1]}] : REC7_TBL[Idx]; - // Output exponent constants for native bias B and internal bias D = P.D_BIAS (e = unbiased input exponent): - // normal input, Xe = e + D: rec B-1-e = (B+D-1) - Xe - // rsqrt (3B-1-(e+B))/2 = ((2B+D-1) - Xe) / 2 + // Supported SEW, one-hot. A flag that is constant 0 removes its format from every select below. + assign Sew16 = P.ZFH_SUPPORTED & (Vsew == VSEW_16); Change to ZVFH once supported in Wally. Currently, ZFH is used to test synthesis size effects. — Reply to this email directly, view it on GitHub <#1858?email_source=notifications&email_token=AR4AA33DRTSFA7O32T6LPJL5VI72RA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKNRRHAZDOMZYGM3KM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-5618273836>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AR4AA3Z4KAPTSG2IHONHSDL5VI72RAVCNFSNUABFKJSXA33TNF2G64TZHMZTEOJYGA3DMMJVHNEXG43VMU5TKNBTGYZTIMZZG4ZKC5QC> . Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/AR4AA3YDZVS73KAKWCBSNNL5VI72RA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKNRRHAZDOMZYGM3KM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android <https://github.com/notifications/mobile/android/AR4AA33CYLEXXBSVD6RUAKL5VI72RA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKNRRHAZDOMZYGM3KM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today! You are receiving this because you commented.Message ID: ***@***.***>