On Fri, 7 Nov 2025 23:34:10 GMT, Shawn M Emery <[email protected]> wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 986:
>>
>>> 984: int idx = kLen - widx;
>>> 985:
>>> 986: dw[idx] = TMI0[w[widx] >>> 24] ^ TMI1[(w[widx] >> 16) &
>>> 0xFF]
>>
>> Do you think there would be any benefit to putting w[widx] through w[widx+3]
>> on local int variables? In some cases I've seen where that increases
>> register pressure and can lead to some perf benefits. I'm not sure if this
>> is one of those cases but it seems like you'd only need to reference memory
>> once instead of 4 times per assignment.
>
> I believe my original changes here utilize a "MergeStore" technique that the
> compiler optimizes. I've asked @minborg to see if I got this right. To
> verify the optimization here, I used the separate local int variable
> technique and saw a 0.7% decrease in benchmark performance.
Well, your experiment answers my question. Unless @minborg says otherwise I'm
fine with what you have. You're already seeing improvements so that's great.
>> src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 1011:
>>
>>> 1009: */
>>> 1010: private static int subWord(int word) {
>>> 1011: byte b0 = (byte) (word >>> 24);
>>
>> Nits (unrelated to this block of code, but I can't put comments on the lines
>> directly since they're not part of the modified code):
>>
>> - Lines 37-43: I think you could do an `@link` annotation with <a href></a>
>> surrounding the links.
>> - Lines 1004-1005: Looks like a bit of comment rot, I think you just need an
>> `@param` for `word`.
>
> Re: `@link` - I'm not for sure I understand the context and couldn't find an
> example of this in the existing code base.
> Re: `@param` - Good catch. I've made this update and a couple of others that
> needed cleaning up.
When I was looking at the code in IntelliJ it was giving warnings about the
links, but I think it just wanted them to be inside href HTML tags so the links
would be active when rendering the comment block. After looking at the `@link`
tag in reference docs I think it's the wrong tag. Honestly, let's just forget
this - if it was a public-facing javadoc comment it would be more important but
what you have here is fine.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28188#discussion_r2507020546
PR Review Comment: https://git.openjdk.org/jdk/pull/28188#discussion_r2507019920
