On Tue, 4 Feb 2025 18:57:28 GMT, Ferenc Rakoczi <d...@openjdk.org> wrote:
>>> @ferakocz I'm afraid you lucked out on getting your change committed before >>> my reorganization of the stub generation code. If you are unsure of how to >>> do the merge so your new stub is declared and generated following the new >>> model (see the doc comments in stubDeclarations.hpp for details) let me >>> know and I'll be happy to help you sort it out. >> >> @adinn I think I managed to figure it out. Please take a look at the PR and >> let me know if I should have done anything differently. > >> @ferakocz Yes, the stub declaration part of it looks to be correct. >> >> The rest of the patch will need at least two reviewers (@theRealAph? >> @martinuy? @franferrax) and may take some time to review, given that they >> will probably need to read up on the maths and algorithms. As an aid for >> reviewers and maintainers it would be good to insert a comment into the >> generator file linking the implementations to the relevant maths and >> algorithm. I found the FIPS-204 spec and the CRYSTALS-Dilithium Algorithm >> Specifications and Supporting Documentation paper, Shi Bai, Léo Ducas et al, >> 2021 - are they the best ones to look at? > > The Java implementation of ML-DSA is based on the FIPS-204 standard and the > intrinsicss' implementations are based on the corresponding Java methods, > except that the montMul() calls in them are inlined. The rest of the > transformation from Java code to intrinsic code is pretty straightforward, so > a reviewer need not necessarily understand the whole mathematics of the > ML-DSA algorithms, just that the Java and the corresponding intrinsic code do > the same thing. @ferakocz Apologies for the delays in reviewing and the limited feedback up to now. The code clearly does the job well but I think it would be made clearer and easier to maintain by tweaking/extending some of the generator methods and adding more detailed commenting. I am afraid I may take a few days to provide the relevant details because of other commitments. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23300#issuecomment-2668251335
