On Wed, 6 May 2026 08:03:41 GMT, Aleksey Shipilev <[email protected]> wrote:

>> Andrew Haley has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 111 commits:
>> 
>>  - Merge branch 'master' into JDK-8134940
>>  - Review comments
>>  - Review comments
>>  - Add FrameMap::last_fpu_reg()
>>  - Review comments
>>  - Review comments
>>  - Review comments
>>  - Merge branch 'JDK-8134940' of https://github.com/theRealAph/jdk into 
>> JDK-8134940
>>  - More
>>  - Merge branch 'JDK-8134940' of https://github.com/theRealAph/jdk into 
>> JDK-8134940
>>  - ... and 101 more: https://git.openjdk.org/jdk/compare/3e1d0b8e...4610777b
>
> src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp line 2918:
> 
>> 2916:         } else {
>> 2917:           __ jmp(*overflow_stub->entry());
>> 2918:           goto exit;
> 
> Do you care about `goto` here? Sounds like `return` would suffice, unless I 
> am missing something?

Indeed so; it's simpler than it was.

> src/hotspot/cpu/x86/c1_LinearScan_x86.hpp line 65:
> 
>> 63: inline bool LinearScanWalker::pd_init_regs_for_alloc(Interval* cur) {
>> 64:   int last_xmm_reg = pd_first_xmm_reg + 
>> XMMRegister::available_xmm_registers()
>> 65:     - (UseVregsForProfileCapture ? 2 : 0) - 1;
> 
> With AVX-512, subtracting two last registers reserve `xmm30/xmm31`, not 
> `xmm14/xmm15` that we are using for counters. So profiling code would stomp 
> the real registers on AVX-512?

I took the vectorized profile counters out.

> src/hotspot/cpu/x86/c1_MacroAssembler_x86.cpp line 44:
> 
>> 42: #include "utilities/globalDefinitions.hpp"
>> 43: 
>> 44: Register r_profile_rng;
> 
> So in AArch64 this one is `extern`. Should neither/both be `extern`?

Fixed.

> src/hotspot/cpu/x86/c1_MacroAssembler_x86.cpp line 282:
> 
>> 280:   } else {
>> 281:     if (UseVregsForProfileCapture) {
>> 282:       vpmulld(xmm15, xmm15, xmm14, Assembler::AVX_128bit);
> 
> Does this require `vpmulld` `AVX > 0`? Does current code assert/fail with 
> `-XX:UseAVX=0` or some such?

Removed.

> src/hotspot/cpu/x86/c1_MacroAssembler_x86.cpp line 287:
> 
>> 285:        
>> https://www.researchgate.net/publication/2683298_A_Collection_of_Selected_Pseudorandom_Number_Generators_With_Linear_Structures
>>  */
>> 286:       movl(temp, 69069);
>> 287:       imull(state, temp);
> 
> `+1` is missing in this LGC? AArch64 has it.

Indeed. Entacher mentions both versions, with and without the +1. Without is 
shorter code, of course, but if ever there is a missing save/restore _and_ the 
register is set to zero, then we get zeroes forever. I'll put the +1 back.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3334937142
PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3334946059
PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3334962816
PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3334969172
PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3334985046

Reply via email to