On 29/06/15 09:37, Vladimir Kozlov wrote:
> Hi, Andrew
> 
> Did you file RFE for this change?  8046943 is JEP.

No; I will do so.

> typo? "less" -> "more".
> 
> +     * number of ints in the number is less than this value we do not
> +     * use the intrinsic.
> +     */
> +    private static final int MONTGOMERY_INTRINSIC_THRESHOLD = 512;
> 
> trailing spaces:
> src/java.base/share/classes/java/math/BigInteger.java:273: Trailing whitespace
> src/java.base/share/classes/java/math/BigInteger.java:2770: Trailing 
> whitespace
> 
> I ran changes through JPRT and linux/solaris passed - thanks.
> Next step - Windows:
> 
> C:\jprt\T\P1\s\hotspot\src\cpu\x86\vm\sharedRuntime_x86_64.cpp(26) : fatal 
> error C1083: Cannot open include file: 
> 'alloca.h': No such file or directory

Hmm, okay.  This is going to be fun.  :-)

AFAIK OpenJDK builds with Visual Studio.  The VS equivalent of
alloca() is called _alloca() and its header file is <malloc.h>.  I'm
going to try to do this untested.  I think that autoconf will #include
malloc.h on Windows automagically, so all that I have to do is create
a #define for alloca() on Windows.

> I am fine with JDK changes.
> 
> Would be nice to have a test for this change. Do existing tests
> cover this code?

They do.  jdk/test/com/oracle/security/ucrypto/TestRSA looks like a
pretty thorough test.  If you make any mistake in the arithmetic RSA
decryption simply will not work: the result is corrupt.  The risks,
then, are mistakes such as accidental side-effects.  I don't know any
way to test for that.

The other possible think I could test for is unusual key sizes.  I'll
have a look.

> I agree that we should limit size when to invoke multiplyToLen
> intrinsic too. File bug I will assign it.

OK.

Andrew.

Reply via email to