On Wed, 12 Jun 2024 11:59:47 GMT, Emanuel Peter <epe...@openjdk.org> wrote:
>> Shaojin Wen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> revert
>
> Going back to:
>
> ./java -XX:+TraceMergeStores -Xbatch -XX:CompileCommand=printcompilation,*::*
> -XX:+PrintInlining AppendNullTest.java > log.log
>
>
> We have occurances of `TraceMergeStores` in these compilations:
>
> 9139 1992 b 4 AppendNullTest$StringBuilder::appendNull (75 bytes)
> -> as discussed above
>
> 9156 1996 % b 4 AppendNullTest::main @ 11 (45 bytes)
> -> once main method is OSR compiled
>
> 529 StoreB === 513 408 527 464 [[ 554 540 ]] @byte[int:>=0]
> (java/lang/Cloneable,java/io/Serializable):exact+any *, idx=9; Memory:
> @byte[int:>=0] (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any
> *, idx=9; !jvms: AppendNullTest$StringUTF16::putChar @ bci:15 (line 115)
> AppendNullTest$StringUTF16::putCharsAt @ bci:3 (line 87)
> AppendNullTest$StringBuilder::appendNull @ bci:63 (line 25)
> AppendNullTest::main @ bci:18 (line 123)
> 8 such StoreB get turned into StoreL. This also comes from the recursive
> inlining, ultimately from the same `putChar`.
>
> 1154 StoreB === 1139 3516 1152 464 [[ 1178 1164 ]] @byte[int:>=0]
> (java/lang/Cloneable,java/io/Serializable):exact+any *, idx=9; Memory:
> @byte[int:>=0] (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any
> *, idx=9; !jvms: AppendNullTest$StringUTF16::putChar @ bci:15 (line 115)
> AppendNullTest$StringUTF16::putCharsAt @ bci:3 (line 87)
> AppendNullTest$StringBuilder::appendNull @ bci:63 (line 25)
> AppendNullTest::main @ bci:21 (line 123)
> Now the pattern repeats, because you have multiple `appendNull` calls in the
> `main` melthod.
> So a total of 5 such optimizations, since all the methods are inlined
> multiple times.
>
>
> That is all the optimizations that I see, where `MergeStores` happens. Now
> the question is if there are any compilations where we would expect the
> `MergeStores` to happen but they do not. That is more difficult.
>
> I can for example see a previous compilation of the `main` method:
>
> 9151 1995 b 3 AppendNullTest::main (45 bytes)
> @ 5 AppendNullTest$StringBuilder::<init> (26
> bytes) inline
> @ 1 java.lang.Object::<init> (1 bytes)
> inline
> @ 18 AppendNullTest$StringBuilder::appendNull
> (75 bytes) failed to inline: callee is too large
> @ 21 AppendNullTest$StringBuilder::appendNull
> (75 bytes) failed to inline: callee is too large
> @ 24 AppendNullTest$StringBuilder::appendNull
> (75 bytes) failed to inline: callee is t...
Thank you very much @eme64 for your help and your patience.
I have another question. The StringUTF16.putChar method inside JDK is an
intrinsic method, which seems to cause the putCharsAt method to be unable to
merge to long stores.
Is it possible or necessary for C2 to continue to improve it?
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);
}
}
[TraceMergeStores]: Replace
78 StoreB === 61 7 76 12 [[ 103 89 ]] @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):exact+any *, idx=5; Memory:
@byte[int:>=0] (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *,
idx=5; !jvms: StringUTF16::putChar @ bci:43 (line 71)
103 StoreB === 88 78 101 80 [[ 18 ]] @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):exact+any *, idx=5; Memory:
@byte[int:>=0] (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *,
idx=5; !jvms: StringUTF16::putChar @ bci:52 (line 72)
[TraceMergeStores]: with
12 Parm === 3 [[ 78 66 38 80 107 113 ]] Parm2: int !jvms:
StringUTF16::putChar @ bci:-1 (line 69)
113 StoreC === 88 7 76 12 [[ ]] @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=5;
mismatched Memory: @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=5;
-------------
PR Comment: https://git.openjdk.org/jdk/pull/19626#issuecomment-2162866205
