Improve performance by fusing kernels - #57
Conversation
|
Thanks for this — the fusion and the 16-byte alignment are both well-motivated, the commit history is easy to follow, and the two side fixes (queue properties, microsecond timing) are worth having on their own. I checked that the refactor is behaviour-preserving: the One blocker, then a few smaller asks. Blocker: the
|
|
Hi, @au2001 . Thanks for the PR. |
|
Hi @galekseev, sorry for missing that, should be fixed now I refreshed the README benchmarks too, but wasn't able to get an RX 480 specifically, so I left it at the old value |
|
All four items are closed — thanks for the quick turnaround. I re-verified the updated branch, this time by actually building and running it under PoCL rather than only reading codegen. A correction on my side first. I claimed the bare What I verified on
|
| Generation | VGPR master → PR | Occupancy |
|---|---|---|
| gfx803 (RX 480) | 192 → 202 | 1 → 1 |
| gfx1030 (RDNA2, RX 6000) | 189 → 203 | 5 → 4 |
| gfx1100 (RDNA3, RX 7900 XTX) | 192 → 192 | 8 → 8 |
If anyone has an RX 6000-series card, one run would close this. It is not a merge blocker — the worst case is a regression confined to a single generation, and it would show up in a measurement.
2. The RX 480 row is a pre-fusion number sitting in a table of post-fusion numbers. You said as much in the thread, but the README does not, and the row's memory speed was changed from 2000 to 4000 while the "Modified straps: YES" column was dropped, which makes it read as freshly measured. A footnote would do.
Also worth updating the PR description: it still carries only the NVIDIA table, and "Requires clearing the cache to see changes" understates the effect — without clearing the cache the program does not start at all.
Nothing else came up. The curve constants, RLP prefixes, seed generation and private-key derivation remain untouched, and the result buffer still carries only the address.
Change Summary
What does this PR change?
It improves performance by 8-12% with these compounding changes:
mp_numberto 2x 16 bytes instead of 8x 4 bytes; and(
profanity_inverseis untouched in this PR, so there are still 2 launches total, down from 3-4)Testing & Verification
How was this tested?
I ran the
--leading 0scoring method on different GPUs with the base and head codebase.For each resulting address, I ran an external verification script which found no mistake.
Here are the results for "70" variant of each NVIDIA generation:
Edit: Previous results used
--benchmark, for which the compiler optimized the kernel by deleting the keccak + scoring. A patch has been added to prevent this from happening.Notes:
During benchmarks, when reaching speeds of 2-3ms per round, the millisecond clock precision was insufficient.
I thus bumped it to microseconds for increased accuracy.
A bug also prevented from enabling profiling:
clCreateCommandQueueWithPropertieson OpenCL 2.0+ expects a zero terminated list of name/value pairs, not a bitfield like 1.2 –0just so happened to be interpreted as an empty list.Risk Assessment
Risk Level:
Risks & Impact
Fully backwards-compatible with no user-facing changes.
But changes how the kernels work in a non-trivial way.
Requires clearing the cache to see changes (and to rollback).
Disclaimer: this PR was assisted by Claude Opus 5 for code exploration & benchmark scripts.
However, all code being submitted has been hand-written – and I'm a human :)