On Thu, 30 Apr 2026 21:06:12 GMT, Shawn Emery <[email protected]> wrote:
>> Ferenc Rakoczi has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Response to camments.
>
> src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 6279:
>
>> 6277: // short[] result, short[] ntta, short[] nttb, short[]
>> zetas) {}
>> 6278: //
>> 6279: // The actual algorithm that is used here differs from the one in
>> the Java
>
> nit: copyright update
Fixed.
> src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 54:
>
>> 52: private static final int MONT_DIM_HALF_INVERSE = 1534;
>> 53: private static final int BARRETT_MULTIPLIER = 20159;
>> 54: private static final int BARRETT_ADDEND = 1665;
>
> nit: copyright update
Fixed.
> src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 1155:
>
>> 1153: int b1 = nttb[m + 1];
>> 1154: long r = a1 * b1;
>> 1155: r = r - ((r * BARRETT_MULTIPLIER) >> BARRETT_SHIFT) *
>> ML_KEM_Q;
>
> For consistency, should this be `r -= (r * BARRETT_MULTIPLIER) >>
> BARRETT_SHIFT) * ML_KEM_Q;`?
Fixed.
> src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 1533:
>
>> 1531: for (int m = 0; m < ML_KEM_N; m++) {
>> 1532: tmp = poly[m];
>> 1533: poly[m] = (short) (tmp - ((tmp * BARRETT_MULTIPLIER) >>
>> BARRETT_SHIFT) * ML_KEM_Q);
>
> Can this be simplified to `poly[m] -= (short) ((poly[m] * BARRETT_MULTIPLIER)
> >> BARRETT_SHIFT) * ML_KEM_Q;`?
I prefer it this way. The suggested version requires a compiler optimization
(which I hope is present in the compiler) to avoid reading the array element
twice. The compiled version should be the same for both forms after
optimization.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30991#discussion_r3219207780
PR Review Comment: https://git.openjdk.org/jdk/pull/30991#discussion_r3219210500
PR Review Comment: https://git.openjdk.org/jdk/pull/30991#discussion_r3219220283
PR Review Comment: https://git.openjdk.org/jdk/pull/30991#discussion_r3219260538
