On Wed, 9 Nov 2022 14:27:26 GMT, Ferenc Rakoczi <d...@openjdk.org> wrote:

> The way I see it is this: as limbs are 64-bit wide, the only place where they 
> can possibly overflow (during the computations they are used for) is the 
> multiplication (including multiply by int and squaring). So I would first try 
> to change the mult() and square() methods only in IntegerPolynomialP256.java 
> (well, in the generator that creates it).

I agreed.  The addition and subtraction operations are limited as well.  Each 
limb cannot exceed 32 bits, otherwise the carry/reduce may not work as far as I 
can see.   It would be good the addition and subtraction was placed in 
IntegerPolynomialP256.java as well.  Otherwise, we have to check the the limits 
in the caller level, which is error-prone.

I will think about how to make this suggestion right.  It may be another PR for 
the restructure.

> (It would also be nice to add comments to the various carry/reduce methods 
> that explain what exactly they want to achieve -- although this is definitely 
> something that doesn't have to be in this change set.)

Yes, there are not much comment in the current code and hard to get the ideas.  
I will see if I can add more in another PR.

> I also think (agree with you) that the setReduced() method can be eliminated 
> if you reduce the multiplicands conditionally (if numAdds > maxAdds) before 
> the multiplication/squaring and unconditionally after it (this part is done 
> in the generated functions already). But that assumes you change all 
> subclasses of IntegerPolynomial that way (most conveniently in the 
> setProduct,Square methods).

The plan is to change all curves (curve448 may be an exception, but I still 
investigate on it.  I can see performance improvement for other curves).

-------------

PR: https://git.openjdk.org/jdk/pull/10624

Reply via email to