Improve performance by fusing kernels

[au2001]

Change Summary

What does this PR change?

It improves performance by 8-12% with these compounding changes:

• aligning mp_number to 2x 16 bytes instead of 8x 4 bytes; and
• merging kernels for iterating/transforming/scoring into a single one

(profanity_inverse is untouched in this PR, so there are still 2 launches total, down from 3-4)

Testing & Verification

How was this tested?

• Unit tests
• Integration tests
• Manual testing (describe steps)
• Verified on staging

I ran the --leading 0 scoring 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:

GPU	VRAM	base	head	inverse-multiple	gain
RTX 5070	12 GB	787 MH/s	876 MH/s	-I 393216	+11.3%
RTX 4070	12 GB	659 MH/s	712 MH/s	-I 393216	+8.0%
RTX 3070	8 GB	487 MH/s	536 MH/s	-I 262144	+10.1%
RTX 2070	8 GB	328 MH/s	359 MH/s	-I 16384	+9.4%
GTX 1070	8 GB	201 MH/s	225 MH/s	-I 196608	+12.2%

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: clCreateCommandQueueWithProperties on OpenCL 2.0+ expects a zero terminated list of name/value pairs, not a bitfield like 1.2 – 0 just so happened to be interpreted as an empty list.

Risk Assessment

Risk Level:

• Low - Minor changes, no operational impact
• Medium - Moderate changes, limited impact, standard rollback available
• High - Significant changes, potential operational impact, complex rollback

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 :)

[cursor]

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 doubles bit trick is equivalent for all 256 byte values, profanity_byte() matches the old uchar reinterpretation over random addresses, and the word-wise --exact compare matches the old byte-wise one over random masks built the way Mode::matching builds them. Dropping the pInverse address write is safe too, since profanity_inverse rewrites that whole range every round.

One blocker, then a few smaller asks.

Blocker: the inline helpers need to be static inline

OpenCL C follows C99, where inline without static or extern is an inline definition rather than an external one. If the compiler declines to inline a call, no body is emitted and the symbol is left undefined. That is what happens here with the clang front end that ROCm, PoCL, Intel NEO, Mesa Rusticl and Apple all use:

$ cat keccak.cl profanity.cl | clang -x cl -cl-std=CL1.2 -Xclang -finclude-default-header \
    -c -O2 -DPROFANITY_INVERSE_SIZE=255 -DPROFANITY_MAX_SCORE=40 -o /tmp/pr57.o -
$ nm -u /tmp/pr57.o | grep profanity
                 U profanity_iterate
                 U profanity_score_fn_mirror

Undefined symbols by optimisation level: -O0 → 10 (i.e. all ten helpers), -O1 → 1, -O2 → 2, -O3 → 4. master is clean at every level, so there is no optimisation setting where this is safe. NVIDIA's front end inlines unconditionally, which is why none of your five cards show it.

Adding static to all ten inline functions (profanity_iterate, profanity_byte, and the eight profanity_score_fn_*) removes every undefined symbol at every level, keeps the -cl-std=CL1.2 -Werror build clean, and costs about 7% of emitted .text.

Running tests/ under PoCL is a cheap way to confirm this, since tests/testutil.hpp builds the full keccak.cl + profanity.cl + harness.cl program.

The stale kernel cache is a hard failure, not just stale performance

getDeviceCacheFilename() only encodes inverse-size and device id, so anyone who has run profanity2 before will load a cached binary that has no profanity_iterate_score_* kernel. clCreateKernel then returns NULL and the program exits with failed to create kernel "profanity_iterate_score_leading" instead of running at the old speed. The PR description reads as if the only consequence were not seeing the speedup.

Could you add a README note like the one #56 added? Making the cache key a hash of the sources plus build options would fix this class of problem permanently, but that is a separate PR and not yours to carry.

Please validate on at least one non-NVIDIA device

All five data points are NVIDIA. Fusing raises the amount of live state in a single kernel, and because bContract is a runtime argument the second ethhash stays in the register budget even for plain address runs — occupancy effects there are vendor-specific. The README still carries an RX 480 row and four Apple Silicon rows, and profanity.cl itself warns that the RX 480 is sensitive to changes in this area. One AMD and one Apple Silicon run would be enough to be comfortable.

Smaller things

• pInverse is read-only in the fused kernel now, so __global const mp_number * const may buy a little more on NVIDIA through the read-only path.
• SpeedSample::sample still divides by delta with no zero guard, which yields inf in the displayed speed. Microseconds make that far less likely, but you are touching that line anyway.
• The README "Current version" benchmark table was measured with the old four-launch --benchmark, which no longer means the same thing after the fusion — worth a note or a refresh.
• Minor consistency point: the contract path reads bytes through a uchar* cast while everything else goes through profanity_byte().

Nothing else in the diff concerned me. The curve constants, the RLP prefixes, seed generation and the private-key derivation are all untouched, and the result buffer still carries only the address.

[galekseev]

Hi, @au2001 . Thanks for the PR.
The blocker for merging is that it breaks compatibility with non-NVIDIA devices.
It would be great if you could fix it.

[au2001]

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

[cursor]

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 inline would break ROCm, PoCL, Intel and Apple. I have now tested PoCL 5.0 and it builds and runs the pre-fix version fine — its pipeline links the bitcode and force-inlines everything into the kernels, so the undefined symbol never materialises. The object-level evidence was real, but "will break" was too strong; the accurate statement is that the code was relying on inliner heuristics rather than being well-formed C99. static inline is still the right fix, and the branch now reports zero undefined symbols at every optimisation level, against 10/1/2/4 at -O0/-O1/-O2/-O3 before.

What I verified on aad8e13

• Kernel builds clean under -Werror with -cl-std=CL1.2, CL2.0 and CL3.0. tests/ passes under PoCL. No new host compiler warnings — the two that appear are identical to master.
• End-to-end correctness against an independent pure-Python secp256k1 + Keccak-256 reference that shares no code with the kernel: 863 key/address pairs verified, i.e. the seed private key plus the printed offset re-derives exactly the printed address. That covers --leading, --matching, --exact, --zeros, --letters, --numbers, --range, --leading-range, --mirror, --leading-doubles and --zero-bytes, plus --contract combined with both --leading and --exact, and a chunked launch via -W 4096.
• 32 score values checked against an independent reimplementation of each scoring function. All match, so the refactor into profanity_score_fn_* is behaviour-preserving on real data and not just by inspection.
• 2.2M --exact hits (plain and contract) all satisfy their mask.
• The contract chain verifies in full: private key to sender address, then keccak(0xd6 0x94 addr 0x80) to contract address.
• The stale-cache case reproduces exactly as described. With a cache written by master, the new binary exits with failed to create kernel "profanity_iterate_score_leading", and --no-cache clears it. The README note now at the top is well placed.
• --benchmark matches --leading 0 throughput under PoCL, so the keccak is definitely no longer being eliminated.
• Throughput on CPU is at parity. Measured externally via --exact hit rate rather than the internal counter, three runs each: master 5.13 / 5.02 / 5.31 MH/s, this branch 5.14 / 5.03 / 5.08 MH/s. That is expected — the round trips and launch overhead the fusion removes are cheap on a CPU device.

Incidentally that last measurement justifies the microsecond commit on its own: master reports exactly 8.192 MH/s at -i 32 -I 256, which is 8192 × 1000, i.e. the millisecond timer quantising a sub-millisecond round up to 1 ms.

Two small things left, neither blocking

1. RDNA2 is the remaining gap. Compiling for AMD through LLVM, the fused kernel needs 203 VGPRs against 189 for profanity_iterate, which drops occupancy from 5 to 4 waves on gfx1030. On gfx803 occupancy is already 1 and stays 1, and on gfx1100 registers are unchanged at 192 with occupancy 8 — so your RX 480 and RX 7900 XTX runs both land on generations where nothing changes:

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.

[cursor]

We measured this on rented GPUs with an A/B/A/B harness, and against a fair baseline the branch is faster on every configuration we tested. The harness now lives in master under bench/, and the raw logs are committed in bench-logs/.

The baseline is 9011bcd — the first commit of this branch, which changes only the speed counter — so both sides measure in microseconds and the comparison is about the kernels rather than about the timer fix.

GPU	A: 9011bcd	B: aad8e13 (this PR)	Δ	max spread
1× RTX 3060	302.9 MH/s	329.7 MH/s	+8.9%	0.5%
2× RTX 3060	507.4 MH/s	567.0 MH/s	+11.7%	0.2%
1× RTX 5090	1694.5 MH/s	1801.0 MH/s	+6.3%	0.1%
8× RTX 5090	12878 MH/s	13789 MH/s	+7.1%	0.4%

Workload --leading 0 -i 255 -I 16384 -w 64, two 120-second windows per revision with the first 30 seconds of each run dropped, revisions alternating A B A B on the same GPU, median reported. Run-to-run spread never exceeds 0.5%, so every delta is well clear of the noise, and the range lands right on top of the 8-12% you reported.

Reproducible with bench/build.sh 9011bcd pr/57 followed by docker run --rm --gpus all <image>.

A false alarm worth recording

Our first attempt on the RTX 5090 compared against master rather than 9011bcd, and it appeared to show a 15% regression: master printed 2.089 GH/s against 1.769 GH/s for this branch. That number is not a measurement. With the default work size a round is 255 × 16384 = 4,177,920 keys, and

4177920 × 500 = 2,088,960,000 = 2.089 GH/s

so master printed exactly the value that falls out of delta = 2 ms — its millisecond truncation had saturated. The true round time was therefore somewhere in [2, 3) ms, meaning the true speed was somewhere in (1393, 2089] MH/s. The harness then pinned it at 1694.5 MH/s, which is a 2.47 ms round and duly truncates to 2. So master's honest speed is 1694.5 MH/s and this branch is 6.3% faster, not 15% slower.

Anyone re-testing on a fast card will walk into this, which is why bench/README.md documents it and the harness prints a warning when SpeedSample.cpp differs between the two revisions. It also makes the microsecond commit worth having on its own merits: at 2 GH/s the old counter cannot measure the program at all.

One thing still open: Apple Silicon

On an Apple Silicon Mac the picture is different, and the GPU frequency trace suggests why. Both revisions start at 1400 MHz. 9011bcd settles at roughly 1200 MHz, this branch settles at roughly 900 MHz, and throughput follows the clock down.

That reads like a power ceiling rather than a slower kernel. Fusing removes the idle gaps between four kernel launches and removes the global-memory round trip, so a larger fraction of cycles does arithmetic and sustained power per cycle rises; on a chip whose GPU shares a tight power budget with the CPU, the governor answers by dropping the clock. Per clock the fused kernel is doing more work — it just does not get to keep it.

Two questions, since the README now claims gains on five Apple chips:

1. How long were the Apple runs, and was the machine on AC power (and in High Power mode, if it was a 16-inch MacBook Pro)? Short runs would have captured the boost-clock phase, where this branch is genuinely ahead — which would reconcile both sets of numbers without either being wrong.
2. Would you be willing to redo one Apple measurement as a longer run, say five minutes with the first minute dropped?

We cannot cover this with the container harness, since Docker gets no access to the Apple GPU.

Minor and still outstanding: the RX 480 row in the README is a pre-fusion number, and after the memory-speed edit and the dropped "Modified straps" column it now reads as freshly measured. A footnote would settle it.