[MarkRose]

41 of the 201 rows in fftbpw.h advertise bits-per-word limits the FFTs cannot reach. At the shipped value the roundoff pins at 0.500 and the Gerbicz check fails from the first block, so FFTConfig::maxExp() hands users exponents that cannot complete. Measured on a Radeon RX 7900 XTX; the failures reproduce on an Intel iGPU and on POCL.

Why this is stacked on #34

The values in this PR are only valid with the carryFinal fix, so the branch sits on top of it rather than beside it.

MAXBPW is derived from the table entry, and the kernels branch on it:

#define SLOPPY_MAXBPW   (MAXBPW - 110)
#if ACTUAL_BPW > SLOPPY_MAXBPW

So every exponent running more than 1.10 bpw below its FFT's maximum compiles the sloppy carry — a much wider window than "small bpw" suggests, and one that bestFit puts a large share of real assignments into. That is the path where carryFinal declared i32 tmpCarry while iCARRY is i64, truncating the carry out of the low word. Without #34 the sloppy window fails outright for many of these shapes and a limit cannot be measured at all; with it, every row here was checked 1.25 bpw below its maximum as well as at it, and 51 rows tested in that window came back clean.

The two defects are independent — applying #34 alone rescues none of the 41 rows corrected here — but the numbers depend on it.

That same feedback is why each candidate was measured against a build whose table carried that value. Measuring a candidate against the shipped table tests a kernel that would never ship, and it is not a small effect: on 1:4K:16:1K the naive protocol indicated −4.50 bpw where the correct one gives −1.00.

What was wrong

group	rows	correction
the three FP32 sections	39	−0.10 to −0.40 (median −0.20 for FFT3261/FFT323161, −0.30 for FFT3231)
rows marked // Estimated	2	1:4K:16:1K −1.00, 3:4K:16:1K −0.30

The second group is the tell. FFT3161 and FFT61 are correct in all 32 of their other entries — the only bad row each has is the one nobody measured. Every wrong row in the table is either FP32 or estimated.

The other six estimated rows were measured too, so the marker is gone from all eight: 51:4K:16:1K, 4:4K:16:1K, 50:4K:16:512 and 50:4K:16:1K run clean as shipped and are untouched. 50:1K:16:1K did not — at 11.21 against a measured 9.71 it is the worst row in the file.

Acceptance criterion

All three of: clean at the top of the range, clean 1.25 bpw below it, and top-of-range ROEmax ≤ 0.45 — the level the rows that are already correct reach under the same protocol, rather than a threshold picked for this patch.

It does real work: 2:1K:16:512 at 29.38 and 4:1K:16:1K at 44.60 both ran clean and were still rejected, at ROEmax 0.474 and 0.455; the next step down was taken in each case.

How far each was tested

• All rows: located by ladder (3 exponents × 4000 iterations at both points), then re-verified at 50 000 iterations × 3 exponents.
• 33M and 134M-word shapes (*:4K:16:*, *:1K:16:1K): 20 000 iterations × 1–2 exponents — a single run there is 45 minutes and ~10 GB of VRAM.
• Confirmations of the estimated rows: 1:4K:16:1K 2/2 clean ROEmax 0.092; 3:4K:16:1K 1/1 clean ROEmax 0.273; 51:4K:16:1K clean ROEmax 0.208.

Across all 54 FP32 rows the corrected group's roundoff sits at median 0.428 against 0.445 for the rows left untouched — slightly more conservative than the entries that were already right.

Those untouched rows were verified at the same length rather than assumed, which caught one of mine: 2:1K:16:512 passed a 4000-iteration screen and failed at 50 000, so it is corrected here.

Caveats

• One GPU for the tuning, with the failures cross-checked on two other OpenCL implementations.
• The ROE bar is calibrated to this measurement protocol; a different run length shifts the numbers, which is why the test depth is stated per row above.

🤖 Generated with Claude Code

[MarkRose]

@gwoltman This is ready for your eyes now. All the values have been tested.

[gwoltman]

This commit concerns me. It looks like a performance regression (probably very minor) of doing 64-bit math when 32-bit math would work in most cases. Would a change along the lines of:
#if EXP/NWORDS > 24
i32 tmpCarry
#else
iCARRY tmpCarry;

endif

work? I'm not sure I follow which cases work and don't work to know if something like the above is do-able.

[MarkRose]

Which cases need the 64-bit temporary, precisely (this is the #34 commit; happy to move the discussion there):

carryFinal computes x = u.x + inCarry and re-normalizes it with carryStepSignedSloppy(i64 x, <tmpCarry type>*). With i32 tmpCarry that overload has three branches, and only one of them can overflow:

• EXP / NWORDS >= 32: the carry is taken straight from hi32(x) shifted right, which is exact for any i64 x. i32 is safe.
• non-sloppy (ACTUAL_BPW > SLOPPY_MAXBPW, i.e. within 1.1 bpw of the table limit) and the plain nBits < 32 fallback: the words are normal-sized, so the carry is about sqrt(N)·2^nBits, far inside the 2^(31+nBits) bound of a 32-bit carry. i32 is safe.
• the sloppy 32-bit-word branch (EXP / NWORDS == 31, or >= 23 with SLOPPY_MAXBPW >= 3200; the i96 overload that produces the pair's second word uses >= 22): every word is now up to 2^31 whatever the bpw, so the convolution sums grow to sqrt(N)·2^62 and the carry into carryFinal to about sqrt(N)·2^(62−nBits). The 32-bit carry needs |x| < 2^(31+nBits), so the margin is 2·nBits − 32 − log2(N)/2 bits. That gives −1 at bpw 20 and +1 at 21 for 256K words, which is exactly the 21-fails / 22-passes boundary measured on the XTX, and it explains the old "fails if BPW is too low" comment. At the >= 23 guard the margin is +5 bits at 256K words but only about +1 bit at 128M words, before the tail of a multi-day run — which is why Use a full-width temporary carry in carryFinal (fixes "sloppy fails at low BPW") #34 widened the carry rather than tune the guard.

So your conditional is the right idea with the condition the other way round: i32 everywhere except when the sloppy 32-bit words are in play. In carryinc.cl that would be

#if CARRY64 && EXP / NWORDS >= 22 && EXP / NWORDS <= 31 && SLOPPY_MAXBPW >= 3200 && ACTUAL_BPW <= SLOPPY_MAXBPW
  iCARRY tmpCarry;      // sloppy 32-bit words: the carry into carryFinal can exceed 32 bits
#else
  i32 tmpCarry;
#endif

(Only the integer NTT types with maxbpw >= 33.1 — FFT3161, FFT323161, FFT6431 and four FFT3261 shapes — ever satisfy the first form; FFT64 never reaches it.)

Whether the conditional is worth its complexity depends on whether the i64 temporary costs anything measurable; the extra work is a few 64-bit integer ops per word pair, once per carryFused call. I'm timing master vs #34 vs the conditional above on the XTX now (FFT3161 at high bpw where they differ, plus an FFT64 control for the noise floor) and will post the numbers here.

[MarkRose]

Timing done on the RX 7900 XTX (gfx1100, ROCm), all on 1K:8:256 (4M words): A = current master (i32 tmpCarry), B = #34 (iCARRY), C = the conditional above. Eight interleaved rounds of 20,000 iterations each; the card was shared with a -ztune job, so the statistic is the median of each run's clean samples (within 2% of the run's own minimum), then the median across rounds. Residues agree across A, B and C at every log point on every shape.

carryFused ISA (registers identical in all builds: 111 VGPRs FFT3161, 92 FFT61, no spills):

shape	kernel	A insns	B insns	C insns
FFT3161 38.96 bpw (sloppy off)	main / ROE	9203 / 9418	9203 / 9386	= A
FFT3161 28.00 bpw (sloppy-32 window)	main / ROE	9164 / 9376	9193 / 9377	= B
FFT61 23.34 bpw (sloppy off)	main / ROE	5319 / 5505	5303 / 5489	= A

The 64-bit temporary adds instructions only inside the sloppy-32 window: +29 on 9164, about 0.3% of the kernel, and that is exactly where it is needed for correctness, so the conditional cannot avoid it. Outside the window the i64 version compiles to the same or fewer instructions than i32 (the compiler folds the widening into the 64-bit add it does anyway).

Wall clock, clean-median µs/iteration and paired B/A ratio (median [min..max] over rounds):

shape	A	B	C	B/A
FFT3161 38.96 bpw	728.5	728.5	729.4	0.9995 [0.996..1.010]
FFT3161 28.00 bpw	728.1	727.1	727.9	0.9993 [0.995..1.005]
FFT64 18.00 bpw (control, identical code)	735.6	736.0	735.5	1.0004 [0.995..1.004]
FFT61 23.34 bpw	518.4	517.8	518.1	0.9991 [0.995..1.002]

The control shape (same code in all three builds) puts the noise floor at about 0.2% on the median; no shape's difference exceeds it.

So the i64 temporary costs nothing measurable, and the conditional would only re-select i32 where i32 has no advantage. I'd leave #34 as it is. If you'd still prefer the conditional for clarity I can push it, but it is a five-term guard buying nothing on this hardware.

[gwoltman]

FYI: I'm enhancing -v to output num registers, shared mem, etc. just like CUDA does. Furthermore, -v 10 will output the assembly code for each kernel. I can test on Radeon VII. Can you test on your AMD cards which are RDNA. I should have the change committed later today.

[MarkRose]

Happy to. Once it's in I'll run the new -v on the RX 7900 XTX (gfx1100, ROCm 7.2) and on the Ryzen 7800X3D iGPU (gfx1036), and post what it prints for a few kernels next to what we got from the code objects via comgr (AMD_COMGR_SAVE_TEMPS + llvm-readelf --notes / llvm-objdump), so you can see whether the register and LDS numbers agree on RDNA.

[MarkRose]

Tested 261020f (-v 10) on the two RDNA devices, ROCm 7.2.4: RX 7900 XTX (gfx1100) and the Ryzen 7800X3D iGPU (gfx1036). Full notes in my tree; the short version:

As committed it prints nothing on either device. With AMD_OCL_BUILD_OPTIONS_APPEND=-save-temps=x this ROCm writes only x_<N>_gfx1100.cl and .i from the compile step; no .s ever appears, and -save-temps is not passed to the link/codegen action. Two changes make it work here: put -save-temps-all in AMD_OCL_LINK_OPTIONS_APPEND (plain -save-temps there fails clLinkProgram with INVALID_LINKER_OPTIONS), which yields one _temp_<N>_gfx1100.{s,so,_linked.bc} per kernel, and let isHashStem (KernelCompiler.cpp:190) accept that name shape as well as all-digits. With that local patch the generated code is unchanged (the -cache binaries are byte-identical with and without -v 10).

With the patch, the parser is accurate. Compared against the loaded code objects (llvm-readelf --notes / llvm-objdump -d on the -cache binaries), for fftP, fftW, fftMiddleIn/Out, tailSquare, tailMul, both carryFused variants, the GF31/GF61 middle and tail kernels, at 256:2:256 and 1K:2:256: VGPRs, LDS bytes and scratch match exactly, the assembly dump has the same instruction count and the same first/last instructions as the disassembly (only trailing s_nop/s_code_end padding differs), and no kernel spills. Two systematic differences: SGPRs come from .amdhsa_next_free_sgpr, which is 2 below .sgpr_count everywhere (VCC not included), and the transpose kernels report 241 VGPRs on gfx1100 (193 on gfx1036) where they use 7 — .amdhsa_next_free_vgpr is the allocation padded for the LDS-limited occupancy (; NumVgprs: 7, NumVGPRsForWavesPerEU: 241); printing ; NumVgprs alongside would avoid that surprise.

Things worth fixing before this goes wide:

1. Data loss. With -v 10, the sweep at KernelCompiler.cpp:198-223 deletes every file in -dir whose stem is all digits, whether or not the compiler produced anything. Reproduced with the unmodified binary: 12345.txt, 2026.log, a file named 42 and an empty directory 999 were deleted, and a user's 1234.s was renamed to transposeIn.s. A per-exponent directory named by a small exponent would go the same way. Listing the directory before and after the compile and only touching new entries would close it.
2. Re-exec vs relative paths. main.cpp changes into -dir (line ~101) before execvp(argv[0], argv) (line 139). With a relative binary path the exec fails and the run continues without the dump; with an absolute binary but a relative -dir the child chdirs again relative to the new cwd and exits with "cannot set current path". Exec'ing /proc/self/exe with -dir made absolute first, or re-exec'ing before the chdir, fixes both. The log header is also printed twice.
3. Clutter. The x_*.cl/.i files are not digit-named, so they are never removed: 34-42 files, about 1.1 MB per run, plus one pair from the -cl-std probe compile on every start even with a warm cache.
4. Occupancy needs a label. It is the compiler's waves per SIMD, whose maximum is 16 on RDNA wave32, 10 on gfx906, 8 on CDNA — so "10" is full on the Radeon VII but not on the XTX (tailMul is 128 VGPRs at 10 here). "waves/SIMD (max N)" would make it unambiguous.
5. Variants indistinguishable. A run prints several carry: and two carryFused: lines with different numbers; the _2/_3 file suffix and the defines (ROE, MUL3) are not shown.
6. Minor: each .s is the whole program (both transpose kernels appear in both files, and find(".amdhsa_kernel " + name) would match carryB for carry if they ever shared a program); with a warm -cache nothing is printed and nothing says the kernels came from the cache; the help text does not mention -v 10; -dump is still parsed but its only use was removed.

gfx1036 behaves the same as gfx1100 for all of this (its wrong residues are a known ROCm cache problem on that APU, unrelated). Happy to send the two-line ROCm fix plus the before/after directory listing for #1 as a PR, or leave it with you since you're in that code right now.

[gwoltman]

Fix this to work on your RDNA GPUs as you see fit EXCEPT you cannot use a subdirectory for kernel compiler output files. That was Claude's first instinct on Radeon VII rocm, but no matter what Claude tried, files were output to the working directory. I recall Mihai filing a bug report along these lines 3 years ago. Alas, Radeon VII support is deprecated in rocm, so I cannot upgrade.

Help does not need -v 10 documented. For developer's (and Claude) only. Remove -dump.

I'm done with this code now, so it's all yours :)

[MarkRose]

Will do. Understood on the working directory: the plan is to snapshot the directory listing before each compile and only ever touch files that appeared during it, so nothing pre-existing can be deleted or renamed, with the link-option form for the current ROCm kept alongside your build-option form so the Radeon VII path is unchanged. Plus the re-exec path fixes, the used-vs-allocated register numbers, an occupancy label, and -dump removed. I'll test on gfx1100 and gfx1036 and send it as a PR.

[MarkRose]

The -v 10 work is up as #123: works on ROCm 7.x (both env forms set, your build-option path kept), a directory snapshot so nothing pre-existing is ever deleted or renamed, re-exec fixed for relative paths and done before the log opens, used vs allocated VGPRs, total SGPRs, occupancy as N/max waves per SIMD, variant-named .s files, and -dump removed. Verified on gfx1100 and gfx1036 against the code objects, plus Intel and POCL for the non-AMD path; the Radeon VII path is unchanged but untested here.

[gwoltman]

Ugh. I (well really Claude) just fixed a bug in Z calculations. If you were using the Z value in your max bpw changes, then they'll need to be redone. If you were basing it on maxROE or avgROE, then I think your work is OK.

[MarkRose]

Ugh. I (well really Claude) just fixed a bug in Z calculations. If you were using the Z value in your max bpw changes, then they'll need to be redone. If you were basing it on maxROE or avgROE, then I think your work is OK.

The acceptance criterion was ROEmax ≤ 0.45 plus clean runs at the top of the range and 1.25 bpw below, and the failures it corrected were roundoff pinned at 0.500 with the Gerbicz check failing from the first block. So we're good there.