RFR: 8333893: Optimization for StringBuilder append boolean & null [v4]

[Shaojin Wen]

On Wed, 12 Jun 2024 14:24:00 GMT, Emanuel Peter <epeter at 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?

class StringUTF16 {
    public static void putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4) {
        int address = Unsafe.ARRAY_BYTE_BASE_OFFSET + (i << 1);
        UNSAFE.putChar(value, address    , c1);
        UNSAFE.putChar(value, address + 2, c2);
        UNSAFE.putChar(value, address + 4, c3);
        UNSAFE.putChar(value, address + 6, c4);
    }
}

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

PR Comment: https://git.openjdk.org/jdk/pull/19626#issuecomment-2163242785