[PPC64LE] poly_tomont: trim dead callee-saved spills (frame 320->224)#1773
[PPC64LE] poly_tomont: trim dead callee-saved spills (frame 320->224)#1773Scottcjn wants to merge 1 commit into
Conversation
mkannwischer
left a comment
mkannwischer
left a comment
There was a problem hiding this comment.
Thanks @Scottcjn. Below some nits. Could you address them first, and then we'll get the code changes reviewed.
| li 11, 208 | ||
| li 12, 224 | ||
| lxvx 32+20, 6, 1 | ||
| lxvx 32+21, 7, 1 | ||
| lxvx 32+22, 8, 1 | ||
| lxvx 32+23, 9, 1 | ||
| lxvx 32+24, 10, 1 | ||
| lxvx 32+25, 11, 1 | ||
| lxvx 32+26, 12, 1 | ||
| li 6, 240 | ||
| li 7, 256 | ||
| li 8, 272 | ||
| li 9, 288 | ||
| lxvx 32+27, 6, 1 | ||
| lxvx 32+28, 7, 1 | ||
| lxvx 32+29, 8, 1 | ||
| lxvx 32+30, 9, 1 | ||
| lxvx 32+23, 6, 1 | ||
| lxvx 32+24, 7, 1 | ||
| lxvx 32+27, 8, 1 | ||
| lxvx 32+28, 9, 1 | ||
| lxvx 32+29, 10, 1 | ||
| lxvx 32+30, 11, 1 | ||
| mtlr 0 | ||
| addi 1, 1, 320 | ||
| addi 1, 1, 224 | ||
| blr | ||
|
|
||
| /* To facilitate single-compilation-unit (SCU) builds, undefine all macros. | ||
| * Don't modify by hand -- this is auto-generated by scripts/autogen. */ | ||
| /* To facilitate single-compilation-unit (SCU) builds, undefine all macros. */ |
There was a problem hiding this comment.
Why was this changed? Please change it back.
| * SPDX-License-Identifier: Apache-2.0 OR ISC OR MIT | ||
| * | ||
| * Written by Danny Tsen <dtsen@us.ibm.com> | ||
| * poly_tomont frame trim (#1722) by Scott Boudreaux. |
There was a problem hiding this comment.
That's the old PR number. I would prefer not linking specific PRs.
We don't usually add names to files. Is it important to keep your name in here?
There was a problem hiding this comment.
If you insist, you can add a copyright line higher in the file
Copyright (c) Scott Boudreaux
|
Thanks @mkannwischer. Nits addressed:
The assembly is unchanged from the reviewed version, so what is left vs main is just the frame trim (320 to 224) and the matching save/restore set. Ready for the code review whenever you are. |
mkannwischer
left a comment
mkannwischer
left a comment
There was a problem hiding this comment.
Thanks for your updates. The last 3 commits are missing the DCO signoff.
Also, could you please squash your 4 commits into one?
Scottcjn
force-pushed
the
ppc64le-tomont-frametrim
branch
from
1f5ff37 to
14e8a39
Compare
|
Done. Squashed to a single commit with the DCO Signed-off-by line. Thanks. |
The prologue saved and restored v20-v30, but the body only clobbers v23, v24, and v27-v30. v20, v21, v22, v25, and v26 were spilled and reloaded but never used: five dead stxvx/lxvx pairs and 80 bytes of frame. This saves only the registers the body touches and trims the frame from 320 to 224 bytes. The Montgomery multiply is unchanged. The mlkem/ copy was regenerated with scripts/simpasm. Closes pq-code-package#1722. Signed-off-by: Scott Boudreaux <scottbphone12@gmail.com>
Scottcjn
force-pushed
the
ppc64le-tomont-frametrim
branch
from
14e8a39 to
d77f500
Compare
|
Thanks @Scottcjn. @bhess, @dannytsen, could either if you provide a review for this change please? |
3 participants
Addresses the inline TODO in
poly_tomont_ppc_asm.S(#1722).The prologue saved and restored v20-v30, but the body only clobbers
v23, v24, and v27-v30. v20, v21, v22, v25, and v26 were spilled and
reloaded but never used: five dead stxvx/lxvx pairs and 80 bytes of
frame. This saves only the registers the body touches and trims the
frame from 320 to 224 bytes. The Montgomery multiply is unchanged.
Validation: func, kat, and unit all pass for ML-KEM-512/768/1024
(opt and no_opt) on a POWER8 S824 with -DMLK_FORCE_PPC64LE -mcpu=power8.
The mlkem/ copy was regenerated with scripts/simpasm.
Supersedes #1752, cleaned up per review feedback. Closes #1722.