On Thu, 23 May 2024 19:54:39 GMT, Scott Gibbons <sgibb...@openjdk.org> wrote:
>> Re-write the IndexOf code without the use of the pcmpestri instruction, only
>> using AVX2 instructions. This change accelerates String.IndexOf on average
>> 1.3x for AVX2. The benchmark numbers:
>>
>>
>> Benchmark Score
>> Latest
>> StringIndexOf.advancedWithMediumSub 343.573 317.934
>> 0.925375393x
>> StringIndexOf.advancedWithShortSub1 1039.081 1053.96
>> 1.014319384x
>> StringIndexOf.advancedWithShortSub2 55.828 110.541
>> 1.980027943x
>> StringIndexOf.constantPattern 9.361 11.906
>> 1.271872663x
>> StringIndexOf.searchCharLongSuccess 4.216 4.218
>> 1.000474383x
>> StringIndexOf.searchCharMediumSuccess 3.133 3.216
>> 1.02649218x
>> StringIndexOf.searchCharShortSuccess 3.76 3.761
>> 1.000265957x
>> StringIndexOf.success 9.186
>> 9.713 1.057369911x
>> StringIndexOf.successBig 14.341 46.343
>> 3.231504079x
>> StringIndexOfChar.latin1_AVX2_String 6220.918 12154.52
>> 1.953814533x
>> StringIndexOfChar.latin1_AVX2_char 5503.556 5540.044
>> 1.006629895x
>> StringIndexOfChar.latin1_SSE4_String 6978.854 6818.689
>> 0.977049957x
>> StringIndexOfChar.latin1_SSE4_char 5657.499 5474.624
>> 0.967675646x
>> StringIndexOfChar.latin1_Short_String 7132.541
>> 6863.359 0.962260014x
>> StringIndexOfChar.latin1_Short_char 16013.389 16162.437
>> 1.009307711x
>> StringIndexOfChar.latin1_mixed_String 7386.123 14771.622
>> 1.999915517x
>> StringIndexOfChar.latin1_mixed_char 9901.671 9782.245
>> 0.987938803
>
> Scott Gibbons has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Addressing review comments
Few suggestions
src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 4250:
> 4248: generate_chacha_stubs();
> 4249:
> 4250: if ((UseAVX == 2) && EnableX86ECoreOpts &&
> VM_Version::supports_avx2()) {
`#ifdef COMPILER2` around this code to exclude JVMCI only case.
src/hotspot/cpu/x86/stubGenerator_x86_64.hpp line 582:
> 580:
> 581: #ifdef COMPILER2
> 582: void generate_string_indexof_stubs(address *fnptrs,
> StrIntrinsicNode::ArgEncoding ae);
Is it possible to make `generate_string_indexof_stubs()` as local static method
in `stubGenerator_x86_64_string.cpp` and pass `StubGenerator*` as argument?
Then you don't to include "opto/intrinsicnode.hpp" here.
src/hotspot/cpu/x86/stubGenerator_x86_64_string.cpp line 2:
> 1: /*
> 2: * Copyright (c) 2023, Intel Corporation. All rights reserved.
2024 year
src/hotspot/cpu/x86/stubGenerator_x86_64_string.cpp line 27:
> 25:
> 26: #include "precompiled.hpp"
> 27: #ifdef COMPILER2
You can exclude this file completely from compilation without this `#ifdef` if
you prefix the name with `c2_`.
There is code in make files to exclude such files:
[JvmFeatures.gmk#L38](https://github.com/openjdk/jdk/blob/master/make/hotspot/lib/JvmFeatures.gmk#L38)
-------------
PR Review: https://git.openjdk.org/jdk/pull/16753#pullrequestreview-2075150606
PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1612352891
PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1612383969
PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1612365050
PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1612375730
