RFR: 8320448: Accelerate IndexOf using AVX2 [v33]

[Scott Gibbons]

On Thu, 23 May 2024 19:02:05 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

>> Scott Gibbons has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix for IndexOf.java on mac
>
> src/hotspot/cpu/x86/stubGenerator_x86_64_string.cpp line 268:
> 
>> 266:   __ cmpq(needle_len_p, 0);
>> 267:   __ jg_b(L_nextCheck);
>> 268:   __ xorq(rax, rax);
> 
> out of curiosity, is there any advantage to using `xorq` instead of `xorl` here?
> 
> https://stackoverflow.com/a/33668295/7707617 suggests that `xorl` might be better, but it's a bit dated now.

Thanks for finding this.  It was ignorance on my part as I thought the xorq would have logic to not emit the REX prefix if not necessary, but it doesn't.  Fixed.

> src/hotspot/cpu/x86/stubGenerator_x86_64_string.cpp line 449:
> 
>> 447:   __ cmpq(r13, NUMBER_OF_CASES - 1);
>> 448:   __ ja(L_smallCaseDefault);
>> 449:   __ mov64(r15, (int64_t)small_jump_table);
> 
> would it make sense to use `lea` here?

It may, but I believe the movq is shorter (although maybe not to r15).  I'll do some experimentation.

> src/hotspot/cpu/x86/stubGenerator_x86_64_string.cpp line 803:
> 
>> 801:     __ movq(index, needle_len);
>> 802:     __ andq(index, 0xf);  // nLen % 16
>> 803:     __ movq(offset, 0x10);
> 
> `movl` or `movptr` would produce a shorter encoding

I tried to be consistent with the whole {q,l} syntax throughout when referring to each symbolic register.  I feel that changing this would ripple through the code.  @sviswa7 what do you think?

> src/hotspot/cpu/x86/stubGenerator_x86_64_string.cpp line 1544:
> 
>> 1542:   }
>> 1543: 
>> 1544:   __ align(8);
> 
> why `8` and not `OptoLoopAlignment` ?

Short answer - because I didn't know there was such a thing as `OptoLoopAlignment`.  I'll change that throughout at the top of my loops.  Thanks.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1612201503
PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1612207461
PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1612216483
PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1612218363