Repository navigation
x86_64 + HOL-Light: Replace rej_uniform_eta2/eta4 intrinsics with hand-written assembly and HOL-Light proofs - #1188
Conversation
f789e8d to
1e78719
Compare
CBMC Results (ML-DSA-87, REDUCE-RAM)Full Results (210 proofs)
|
CBMC Results (ML-DSA-65, REDUCE-RAM)Full Results (210 proofs)
|
CBMC Results (ML-DSA-44, REDUCE-RAM)Full Results (210 proofs)
|
CBMC Results (ML-DSA-65)Full Results (210 proofs)
|
CBMC Results (ML-DSA-44)Full Results (210 proofs)
|
CBMC Results (ML-DSA-87)
Full Results (210 proofs)
|
1e78719 to
7c13c63
Compare
There was a problem hiding this comment.
Mac Mini (M1, 2020) benchmarks (opt)
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
46524 cycles |
46501 cycles |
1.00 |
ML-DSA-44 sign |
131156 cycles |
131159 cycles |
1.00 |
ML-DSA-44 verify |
47325 cycles |
47327 cycles |
1.00 |
ML-DSA-65 keypair |
81696 cycles |
81682 cycles |
1.00 |
ML-DSA-65 sign |
215403 cycles |
215381 cycles |
1.00 |
ML-DSA-65 verify |
79311 cycles |
79309 cycles |
1.00 |
ML-DSA-87 keypair |
132415 cycles |
132436 cycles |
1.00 |
ML-DSA-87 sign |
277406 cycles |
277663 cycles |
1.00 |
ML-DSA-87 verify |
134055 cycles |
134051 cycles |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Mac Mini (M1, 2020) benchmarks (no-opt)
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
113028 cycles |
112820 cycles |
1.00 |
ML-DSA-44 sign |
402081 cycles |
401084 cycles |
1.00 |
ML-DSA-44 verify |
119691 cycles |
119458 cycles |
1.00 |
ML-DSA-65 keypair |
192981 cycles |
192907 cycles |
1.00 |
ML-DSA-65 sign |
650273 cycles |
649936 cycles |
1.00 |
ML-DSA-65 verify |
192931 cycles |
192844 cycles |
1.00 |
ML-DSA-87 keypair |
318799 cycles |
318799 cycles |
1 |
ML-DSA-87 sign |
828743 cycles |
828921 cycles |
1.00 |
ML-DSA-87 verify |
326813 cycles |
326734 cycles |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Arm Cortex-A72 (Raspberry Pi 4) benchmarks (opt)
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
222141 cycles |
223446 cycles |
0.99 |
ML-DSA-44 sign |
614240 cycles |
634283 cycles |
0.97 |
ML-DSA-44 verify |
226725 cycles |
223226 cycles |
1.02 |
ML-DSA-65 keypair |
385352 cycles |
393894 cycles |
0.98 |
ML-DSA-65 sign |
1003734 cycles |
1022240 cycles |
0.98 |
ML-DSA-65 verify |
372158 cycles |
378918 cycles |
0.98 |
ML-DSA-87 keypair |
650198 cycles |
644349 cycles |
1.01 |
ML-DSA-87 sign |
1362778 cycles |
1332260 cycles |
1.02 |
ML-DSA-87 verify |
638012 cycles |
627970 cycles |
1.02 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Arm Cortex-A72 (Raspberry Pi 4) benchmarks (opt)'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.03.
| Benchmark suite | Current: 7c13c63 | Previous: 70fdba0 | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
227496 cycles |
219616 cycles |
1.04 |
ML-DSA-44 verify |
235419 cycles |
214195 cycles |
1.10 |
ML-DSA-87 keypair |
661599 cycles |
635525 cycles |
1.04 |
ML-DSA-87 sign |
1369715 cycles |
1306530 cycles |
1.05 |
ML-DSA-87 verify |
645122 cycles |
616298 cycles |
1.05 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Intel Xeon 4th gen (c7i)
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
43323 cycles |
43311 cycles |
1.00 |
ML-DSA-44 sign |
129973 cycles |
130234 cycles |
1.00 |
ML-DSA-44 verify |
45201 cycles |
45328 cycles |
1.00 |
ML-DSA-65 keypair |
75758 cycles |
75678 cycles |
1.00 |
ML-DSA-65 sign |
213832 cycles |
214141 cycles |
1.00 |
ML-DSA-65 verify |
74325 cycles |
74551 cycles |
1.00 |
ML-DSA-87 keypair |
123264 cycles |
122951 cycles |
1.00 |
ML-DSA-87 sign |
270347 cycles |
270036 cycles |
1.00 |
ML-DSA-87 verify |
120938 cycles |
120608 cycles |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Intel Xeon 4th gen (c7i) (no-opt)
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
91475 cycles |
91650 cycles |
1.00 |
ML-DSA-44 sign |
352301 cycles |
353391 cycles |
1.00 |
ML-DSA-44 verify |
99818 cycles |
100249 cycles |
1.00 |
ML-DSA-65 keypair |
153858 cycles |
153883 cycles |
1.00 |
ML-DSA-65 sign |
572079 cycles |
571341 cycles |
1.00 |
ML-DSA-65 verify |
159979 cycles |
159780 cycles |
1.00 |
ML-DSA-87 keypair |
255344 cycles |
255804 cycles |
1.00 |
ML-DSA-87 sign |
726579 cycles |
726454 cycles |
1.00 |
ML-DSA-87 verify |
264184 cycles |
264128 cycles |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
AMD EPYC 3rd gen (c6a)
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
55494 cycles |
55575 cycles |
1.00 |
ML-DSA-44 sign |
158449 cycles |
159176 cycles |
1.00 |
ML-DSA-44 verify |
57633 cycles |
58038 cycles |
0.99 |
ML-DSA-65 keypair |
96318 cycles |
95925 cycles |
1.00 |
ML-DSA-65 sign |
258646 cycles |
259688 cycles |
1.00 |
ML-DSA-65 verify |
95634 cycles |
96324 cycles |
0.99 |
ML-DSA-87 keypair |
155356 cycles |
154620 cycles |
1.00 |
ML-DSA-87 sign |
323135 cycles |
323315 cycles |
1.00 |
ML-DSA-87 verify |
151250 cycles |
151152 cycles |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Arm Cortex-A72 (Raspberry Pi 4) benchmarks (no-opt)
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
298957 cycles |
309045 cycles |
0.97 |
ML-DSA-44 sign |
1130396 cycles |
1181040 cycles |
0.96 |
ML-DSA-44 verify |
327888 cycles |
338438 cycles |
0.97 |
ML-DSA-65 keypair |
557436 cycles |
569427 cycles |
0.98 |
ML-DSA-65 sign |
1888548 cycles |
1984198 cycles |
0.95 |
ML-DSA-65 verify |
538964 cycles |
570461 cycles |
0.94 |
ML-DSA-87 keypair |
861727 cycles |
894629 cycles |
0.96 |
ML-DSA-87 sign |
2454019 cycles |
2499364 cycles |
0.98 |
ML-DSA-87 verify |
907684 cycles |
918394 cycles |
0.99 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Arm Cortex-A72 (Raspberry Pi 4) benchmarks (no-opt)'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.03.
| Benchmark suite | Current: 7c13c63 | Previous: 70fdba0 | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
312772 cycles |
294913 cycles |
1.06 |
ML-DSA-44 sign |
1193382 cycles |
1147432 cycles |
1.04 |
ML-DSA-87 keypair |
904673 cycles |
847930 cycles |
1.07 |
ML-DSA-87 sign |
2468732 cycles |
2393137 cycles |
1.03 |
ML-DSA-87 verify |
906571 cycles |
874118 cycles |
1.04 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
AMD EPYC 3rd gen (c6a) (no-opt)
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
133692 cycles |
133431 cycles |
1.00 |
ML-DSA-44 sign |
520225 cycles |
519219 cycles |
1.00 |
ML-DSA-44 verify |
146942 cycles |
146686 cycles |
1.00 |
ML-DSA-65 keypair |
223592 cycles |
224467 cycles |
1.00 |
ML-DSA-65 sign |
841919 cycles |
843782 cycles |
1.00 |
ML-DSA-65 verify |
233762 cycles |
234011 cycles |
1.00 |
ML-DSA-87 keypair |
366674 cycles |
367264 cycles |
1.00 |
ML-DSA-87 sign |
1057851 cycles |
1060362 cycles |
1.00 |
ML-DSA-87 verify |
380118 cycles |
381078 cycles |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Graviton2
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
112327 cycles |
112432 cycles |
1.00 |
ML-DSA-44 sign |
354085 cycles |
353829 cycles |
1.00 |
ML-DSA-44 verify |
117505 cycles |
117251 cycles |
1.00 |
ML-DSA-65 keypair |
194501 cycles |
194915 cycles |
1.00 |
ML-DSA-65 sign |
584403 cycles |
584545 cycles |
1.00 |
ML-DSA-65 verify |
193506 cycles |
193060 cycles |
1.00 |
ML-DSA-87 keypair |
320982 cycles |
321247 cycles |
1.00 |
ML-DSA-87 sign |
747665 cycles |
747767 cycles |
1.00 |
ML-DSA-87 verify |
318354 cycles |
318871 cycles |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Graviton4
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
67271 cycles |
67334 cycles |
1.00 |
ML-DSA-44 sign |
198182 cycles |
198212 cycles |
1.00 |
ML-DSA-44 verify |
70155 cycles |
70164 cycles |
1.00 |
ML-DSA-65 keypair |
119526 cycles |
119314 cycles |
1.00 |
ML-DSA-65 sign |
326414 cycles |
326337 cycles |
1.00 |
ML-DSA-65 verify |
116796 cycles |
116812 cycles |
1.00 |
ML-DSA-87 keypair |
196482 cycles |
196615 cycles |
1.00 |
ML-DSA-87 sign |
421379 cycles |
421442 cycles |
1.00 |
ML-DSA-87 verify |
193268 cycles |
192981 cycles |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
AMD EPYC 4th gen (c7a)
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
48126 cycles |
46837 cycles |
1.03 |
ML-DSA-44 sign |
138997 cycles |
140005 cycles |
0.99 |
ML-DSA-44 verify |
49819 cycles |
49295 cycles |
1.01 |
ML-DSA-65 keypair |
82346 cycles |
85296 cycles |
0.97 |
ML-DSA-65 sign |
226618 cycles |
235771 cycles |
0.96 |
ML-DSA-65 verify |
82230 cycles |
83871 cycles |
0.98 |
ML-DSA-87 keypair |
130321 cycles |
132099 cycles |
0.99 |
ML-DSA-87 sign |
281743 cycles |
282238 cycles |
1.00 |
ML-DSA-87 verify |
129649 cycles |
129367 cycles |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Graviton4 (no-opt)
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
127610 cycles |
127745 cycles |
1.00 |
ML-DSA-44 sign |
441047 cycles |
440937 cycles |
1.00 |
ML-DSA-44 verify |
136381 cycles |
136447 cycles |
1.00 |
ML-DSA-65 keypair |
220529 cycles |
220468 cycles |
1.00 |
ML-DSA-65 sign |
714603 cycles |
714745 cycles |
1.00 |
ML-DSA-65 verify |
220923 cycles |
221459 cycles |
1.00 |
ML-DSA-87 keypair |
364675 cycles |
364373 cycles |
1.00 |
ML-DSA-87 sign |
915835 cycles |
915833 cycles |
1.00 |
ML-DSA-87 verify |
370946 cycles |
371331 cycles |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Intel Xeon 3rd gen (c6i)
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
61559 cycles |
61513 cycles |
1.00 |
ML-DSA-44 sign |
188176 cycles |
187961 cycles |
1.00 |
ML-DSA-44 verify |
66304 cycles |
66502 cycles |
1.00 |
ML-DSA-65 keypair |
110399 cycles |
108141 cycles |
1.02 |
ML-DSA-65 sign |
314455 cycles |
311311 cycles |
1.01 |
ML-DSA-65 verify |
111028 cycles |
108776 cycles |
1.02 |
ML-DSA-87 keypair |
173234 cycles |
171604 cycles |
1.01 |
ML-DSA-87 sign |
378684 cycles |
380707 cycles |
0.99 |
ML-DSA-87 verify |
171059 cycles |
172479 cycles |
0.99 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Graviton2 (no-opt)
Details
| Benchmark suite | Current: 237e387 | Previous: 31300ad | Ratio |
|---|---|---|---|
ML-DSA-44 keypair |
212157 cycles |
211410 cycles |
1.00 |
ML-DSA-44 sign |
761446 cycles |
758945 cycles |
1.00 |
ML-DSA-44 verify |
229897 cycles |
228936 cycles |
1.00 |
ML-DSA-65 keypair |
378864 cycles |
377838 cycles |
1.00 |
ML-DSA-65 sign |
1248060 cycles |
1248443 cycles |
1.00 |
ML-DSA-65 verify |
373372 cycles |
372800 cycles |
1.00 |
ML-DSA-87 keypair |
602524 cycles |
600578 cycles |
1.00 |
ML-DSA-87 sign |
1584621 cycles |
1583512 cycles |
1.00 |
ML-DSA-87 verify |
618342 cycles |
616106 cycles |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
df464ba to
44c3af7
Compare
mkannwischer
left a comment
There was a problem hiding this comment.
Thanks @jakemas for the improvements.
This proof looks good to me now (except for the s2n-bignum pin - tagging as DO-NOT-MERGE accordingly so this does not get merged accidentally).
@hanno-becker could you please take a look at this proof too?
There was a problem hiding this comment.
Thanks @jakemas, it looks like a massive effort! Kudos for getting it through.
I am bit concerned and certainly surprised about the size-creep in our proofs. The AArch64 proofs for the rejection sampling routines are ~3kLOC, but for x86_64 we're looking at 7kLOC for rej_uniform and now 10kLOC for rej_uniform_eta{2,4}.
Why are these proofs so long, and esp. so much longer than for AArch64? There must be something to learn here... either they're unnecesarily complex -- in which case we should simplify them -- or something is fundamentally more difficult (both between AArch64 and x86_64, and between rej_uniform and rej_uniform_eta{2,4}) -- in that case, too, it would be good to know what that is.
Right, I agree they are big. I thought the same thing, and have been through rounds of parametrization, clean up, optimization. One note, in the last PR you asked for more comments and descriptions of the larger proofs, so there are ~2000 lines of comments in each of those proof files. I'll do another pass now and look for ways this can be size reduced/optimized. |
44c3af7 to
c4987d6
Compare
|
ok was able to make some large reductions:
|
Thanks, these are welcome simplifications! Do you have an intuition why the proofs are more sizable than the corresponding Arm proofs? At this point, I think we shouldn't block this PR due to proof complexity concerns (unless there are soundness concerns related to it). To me, getting v1 out is much more important. However, understanding what complexity is inherent to the code and what can be eliminated with more time would be good. Maybe we can open some follow-up issues. @jakemas, do you know when we can expect the s2n-bignum PR to be merged? Is there anything I can help with? |
5901af9 to
fcd0999
Compare
|
I just pushed a new version, cleaning up the eta2 proof a little more and speeding it up slightly.
Yeah, a few things compound here, and it's mostly instruction-set semantics rather than the algorithm, both sides run the same 256-entry mask-indexed lookup-table method (Arm via TBL, x86 via PSHUFB). I think the lane-crossing in AVX2 is the big one. Arm's TBL does a full cross-register 128-bit table lookup in one instruction, so there's no per-lane decomposition to reason about. More generally, in my experience the x86 proofs have consistently been more involved to write than their Arm counterparts, it feels like some of this gap is inherent to the platform rather than to this routine.
I agree we shouldn't block on this, i'm happy to follow-up issues to track which of the remaining complexity is inherent vs. reducible with more time.
As for the instruction PR, I pinged John today to remind him to review. I also asked if he could take a look at this PR and check out the eta4/2 proofs to see if he can spot any further optimizations, or simpler tactics to use. I can follow up with any suggestion John may make, I'll also PR into s2n-bignum once we're merged here (I don't want to PR any asm that isn't stable here into s2n-bignum), so John will get a look at it then in review and may have ideas. But also, as I said, Hanno asked for more comments on the complex proofs, so there are 1000-2000 lines of comments, the files are now 9000 and 8000 lines, so we're at the ~7k bar from uniform_rej. |
…d-written assembly and HOL-Light proofs Replaces the rej_uniform_eta2 and rej_uniform_eta4 AVX2 intrinsics with hand-written assembly, and adds HOL-Light functional-correctness and memory-safety proofs for both x86_64 routines, plus the CBMC proof for the native wrapper. Both routines are variable-time, so — as for the AArch64 eta proofs — only functional correctness and memory safety are proven, not constant time. The table is passed as a parameter and all constants are built from immediates (no .rodata), following the rej_uniform approach in #1014. Signed-off-by: Jake Massimo <jakemas@amazon.com>
fcd0999 to
e249c22
Compare
|
ok #421 is merged, updated nix hash. @mkannwischer :) |
Amazing! Thanks - looks good. I've removed the |
hanno-becker
left a comment
There was a problem hiding this comment.
LGTM.
This is a milestone. Many thanks @jakemas for all your hard work and perseverance in providing the HOL-Light proofs and navigating our requests!
rej_uniform_etawith assembly #419rej_uniform_eta#917Summary
Replaces the AVX2 intrinsics implementations of
rej_uniform_eta2andrej_uniform_eta4with hand-written x86_64 assembly, and adds HOL-Light functional-correctness and memory-safety proofs for both routines. Follows the same approach as #1014:.rodata), enabling HOL-Light formal verification.__contract__annotations on the asm entry points (CBMC), kept in sync with the HOL-Light specs.meta.hwires both eta2 and eta4 to the new asm, so the functional test suite exercises both paths.Makefileanddump_bytecode.mlregister the eta2/eta4 bytecode dump targets; ABI-checker tests added for both.poly_uniform_eta_4xcomponent benchmark.s2n-bignum dependency
The proofs use the
VPBROADCASTWinstruction, added in awslabs/s2n-bignum#421. Until that merges, the nix pin (nix/s2n_bignum/default.nix) is temporarily pointed at thejakemas/s2n-bignumfork branch so CI can decode the instruction; it will be repointed toawslabsmain once #421 lands.