RFR: 8349452: Fix performance regression for Arrays.fill() with AVX512 [v10]

Srinivas Vamsi Parasa sparasa at openjdk.org
Wed Dec 24 22:06:48 UTC 2025


On Thu, 18 Dec 2025 23:53:08 GMT, Derek White <drwhite at openjdk.org> wrote:

> No correctness issues, but some cleanups suggested. Thanks!

Thank you, Derek (@dwhite-intel) for your review comments and suggestions! Please see the updated code incorporating your suggestions.

> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 5800:
> 
>> 5798:                                    Register rtmp, Register rtmp2, XMMRegister xtmp) {
>> 5799:   ShortBranchVerifier sbv(this);
>> 5800:   assert_different_registers(to, value, count, rtmp);
> 
> Should add rtmp2 to assert_different_registers also

Thanks for catching that! Please see this fixed in the updated code.

> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 9339:
> 
>> 9337:   Label L_exit;
>> 9338:   Label L_fill_start;
>> 9339:   Label L_fill_32_tail;
> 
> This is a bit more style/readability suggestion:
> I think these labels can move to the block starting on line 9387
> 
> L_fill_start, L_fill_32_tail, L_fill_96_bytes, 
> L_fill_128_bytes, 
> L_fill_128_bytes_loop, 
> L_fill_128_bytes_loop_header, 
> L_fill_128_bytes_loop_pre_header

Please see these labels moved to the appropriate places as suggested to improve readability.

> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 9340:
> 
>> 9338:   Label L_fill_start;
>> 9339:   Label L_fill_32_tail;
>> 9340:   Label L_fill_64_tail;
> 
> Similar suggestion: L_fill_64_tail can move to block starting on line 9462

Please see the label L_fill_64_tail moved as suggested.

> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 9341:
> 
>> 9339:   Label L_fill_32_tail;
>> 9340:   Label L_fill_64_tail;
>> 9341:   Label L_fill_64_bytes;
> 
> L_fill_64_bytes is now unused

True, L_fill_64_bytes is now unused is unused now. Please see it removed in the updated code.

> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 9345:
> 
>> 9343:   Label L_fill_128_bytes;
>> 9344:   Label L_fill_128_bytes_loop;
>> 9345:   Label L_fill_128_loop_header;
> 
> L_fill_128_bytes_loop_header is unused (preexisting?)

L_fill_128_bytes_loop_header is a preexisting label and is being used.

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

PR Comment: https://git.openjdk.org/jdk/pull/28442#issuecomment-3690547891
PR Review Comment: https://git.openjdk.org/jdk/pull/28442#discussion_r2646294220
PR Review Comment: https://git.openjdk.org/jdk/pull/28442#discussion_r2646296508
PR Review Comment: https://git.openjdk.org/jdk/pull/28442#discussion_r2646296960
PR Review Comment: https://git.openjdk.org/jdk/pull/28442#discussion_r2646294502
PR Review Comment: https://git.openjdk.org/jdk/pull/28442#discussion_r2646295332


More information about the hotspot-dev mailing list