This corrects two statements in my review of #1609 and adds the benchmark numbers the review asked for. I can no longer comment on or edit anything in that PR's thread, so I am recording the correction here.
CI was not blocked by the base. My review said the branch showed no checks because its base predates the checkasm integration. That was wrong. pull_request workflows run on the merge with master, so checkasm applied to this PR from the start. The runs were created at 06:00 UTC on 2026-09-21, waited for maintainer approval, ran at 17:52 UTC, and all 13 checks pass. The branch still predates 03b5562, but it merges cleanly and CI tested the merge, so a rebase is tidiness rather than a requirement.
The change is a measurable gain, not only a readability change. I benchmarked only adm_decouple_avx2. blend() is also called 48 times in adm_decouple_s123_avx2, where the switch to vpblendvb is the only change. The checkasm job for #1609 ran on an EPYC 7763 (Zen 3). Eight other EPYC 7763 runs of code that leaves both functions unchanged (master and seven PRs) give the baseline:
| function |
other runs, cycles (min to max) |
#1609 |
adm_decouple_64x48_avx2 |
14890 to 15320 |
14578 |
adm_decouple_65x49_avx2 |
13659 to 14129 |
13197 |
adm_decouple_s123_64x48_avx2 |
34267 to 34889 |
32102 |
adm_decouple_s123_65x49_avx2 |
37195 to 37859 |
34122 |
Against the medians that is 3 to 5% for adm_decouple and 7 to 9% for adm_decouple_s123. The C references in the same run are within about 2% of the other runs. On Zen 3, vpblendvb is a gain in its own right, and my suggestion to call #1609 a readability change undersold it. Intel is still measured only for adm_decouple.
One smaller correction: the lo/hi interleave replaces and + or with one vpblendd and keeps the shift. It goes from three operations to two, not from three to one.
Sources: the checkasm bench summaries of CI run 35566639608 (#1609) and of runs 35156048283, 36379521611, 36173280090, 35900963541, 35326804000, 35147520407, 35034617612 and 35034422831. None of those eight changes the decouple code.
This corrects two statements in my review of #1609 and adds the benchmark numbers the review asked for. I can no longer comment on or edit anything in that PR's thread, so I am recording the correction here.
CI was not blocked by the base. My review said the branch showed no checks because its base predates the checkasm integration. That was wrong.
pull_requestworkflows run on the merge with master, so checkasm applied to this PR from the start. The runs were created at 06:00 UTC on 2026-09-21, waited for maintainer approval, ran at 17:52 UTC, and all 13 checks pass. The branch still predates 03b5562, but it merges cleanly and CI tested the merge, so a rebase is tidiness rather than a requirement.The change is a measurable gain, not only a readability change. I benchmarked only
adm_decouple_avx2.blend()is also called 48 times inadm_decouple_s123_avx2, where the switch tovpblendvbis the only change. The checkasm job for #1609 ran on an EPYC 7763 (Zen 3). Eight other EPYC 7763 runs of code that leaves both functions unchanged (master and seven PRs) give the baseline:adm_decouple_64x48_avx2adm_decouple_65x49_avx2adm_decouple_s123_64x48_avx2adm_decouple_s123_65x49_avx2Against the medians that is 3 to 5% for
adm_decoupleand 7 to 9% foradm_decouple_s123. The C references in the same run are within about 2% of the other runs. On Zen 3,vpblendvbis a gain in its own right, and my suggestion to call #1609 a readability change undersold it. Intel is still measured only foradm_decouple.One smaller correction: the lo/hi interleave replaces
and+orwith onevpblenddand keeps the shift. It goes from three operations to two, not from three to one.Sources: the checkasm bench summaries of CI run 35566639608 (#1609) and of runs 35156048283, 36379521611, 36173280090, 35900963541, 35326804000, 35147520407, 35034617612 and 35034422831. None of those eight changes the decouple code.