On Wed, 12 Jun 2024 14:24:00 GMT, Emanuel Peter <epe...@openjdk.org> wrote:
>> _putCharStringU will perform MergeStore but with bounds checking. Can the C2
>> optimizer perform MergeStore without bounds checking in this case?
>>
>> StringUTF16.putChar does not have a bounds check, so there should be no
>> bounds check after MergeStore?
>>
>>
>> class AbstractStringBuilder {
>> private AbstractStringBuilder appendNull() {
>> // ...
>> StringUTF16.putCharsAt(val, count, 'n', 'u', 'l', 'l');
>> // ...
>> }
>> }
>>
>> class StringUTF16 {
>> public static void putCharsAt(byte[] value, int i, char c1, char c2,
>> char c3, char c4) {
>> putChar(value, i , c1);
>> putChar(value, i + 1, c2);
>> putChar(value, i + 2, c3);
>> putChar(value, i + 3, c4);
>> }
>>
>> @IntrinsicCandidate
>> // intrinsic performs no bounds checks
>> static void putChar(byte[] val, int index, int c) {
>> assert index >= 0 && index < length(val) : "Trusted caller missed
>> bounds check";
>> index <<= 1;
>> val[index++] = (byte)(c >> HI_BYTE_SHIFT);
>> val[index] = (byte)(c >> LO_BYTE_SHIFT);
>> }
>> }
>
> @wenshao If you look at the tests in
> https://github.com/openjdk/jdk/pull/16245, you can see examples like this:
> https://github.com/eme64/jdk/blob/93bf2ddc9b7f584724034aec6a4f8b9fe1b2dfda/test/hotspot/jtreg/compiler/c2/TestMergeStores.java#L501-L514
> So yes, we can merge them.
> @eme64 It seems like MergeStore didn't happen, is there something I did wrong?
Yes ;)
@wenshao The issue is that the pattern matching is quite **limited**.
`putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4)`
This has 4 variables. This is not allowed for the optimization. You would have
to pass in a `long` and split it via shifts.
A possible exception: if `putCharsAt` is inlined and outside it is a splitting
of a `long` value with shift, then ... maybe ... this would be optimized with
`MergeStores`.
There are only 2 **accepted** patterns:
- Multiple stores of constants -> constant is merged, which allows stores to be
merged.
- Multiple stores which all store a "section" of a larger value:
You can see **examples** I listed at the beginning of the PR description in
https://github.com/openjdk/jdk/pull/16245.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/19626#issuecomment-2163261884
