On Wed, 7 Jan 2026 18:59:16 GMT, Ferenc Rakoczi <[email protected]> wrote:

>> Yep, thats exactly the assert I was looking at as well.. looks to me like 
>> its dividing the 'expanded-short-array-length' by 64 and ensuring the 
>> remainder is zero (ignoring the 48 for a bit.. and the condensed-length 
>> check).
>> 
>> (for simplicity) So the 'expanded' array length should be a multiple of 64; 
>> i.e. 128-bytes. But we stride the expanded array by 256 bytes? (and 
>> parsedLength by 128-shorts..)
>> 
>> I haven't checked the callers of `twelve2Sixteen` but I suspect that the 
>> length of the expanded array is always a multiple of 256-bytes 
>> (128-shorts).. in which case, the assert is 'incomplete'?
>
> Oooops, yes, the assert and the comment on twelve2sixteen() should be fixed. 
> All of the calls are processing 192 or 384 bytes (and producing 128 or 256 
> shorts). The comment and assert belonged to an earlier version and were not 
> updated when I changed my mind about the implementation.

I've filed bug:

[JDK-8374755](https://bugs.openjdk.org/browse/JDK-8374755) ML-KEM's 12-bit 
decompression uses incorrect assertions

to track this issue.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28815#discussion_r2670893072

Reply via email to