On Sun, 23 Nov 2025 05:23:04 GMT, Jiangli Zhou <[email protected]> wrote:

>> src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp line 3531:
>> 
>>> 3529:   __ subl(len, 16 * 16);
>>> 3530:   __ cmpl(len, 16 * 16);
>>> 3531:   __ jcc(Assembler::lessEqual, ENC_DEC_DONE);
>> 
>> I think the fix should instead be to just move the addl to pos before the 
>> MESG_BELOW_32_BLKS, as below:
>> 
>> +  __ addl(pos, 16 * 16);
>>    __ bind(MESG_BELOW_32_BLKS);
>>    __ subl(len, 16 * 16);
>> -  __ addl(pos, 16 * 16);
>> 
>> This is because on fall through path addl is needed but not while coming 
>> from line 3479 via jcc. For the latter, the addl has already been done on 
>> line 3477.
>
> Hmmm, I think you are correct. Examining the entire flow today, I see 
> `ENCRYPT_16_BLKS` doesn't increment `pos`. Thanks for pointing out that! 
> Before my change, add `pos` was done when it fell through to 
> `MESG_BELOW_32_BLKS`.
> 
> Removed the `cmpl/jcc` change from `MESG_BELOW_32_BLKS` and moved `addl` to 
> above `MESG_BELOW_32_BLKS`, as suggested.
> 
> I did a bit debugging for the rare failures occurring to the new test case. 
> One of the failures had message with length `1048833`. That would eventually 
> go through `ENCRYPT_16_BLKS` then fall through `MESG_BELOW_32_BLKS`. With the 
> updated fix, 200 runs all pass on AVX512 machines. So the rare failures were 
> also related to this then.

It looks good to me now. Please close JDK-8372364 as it was an artifact of the 
prior fix.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28363#discussion_r2557130932

Reply via email to