On Fri, 14 Nov 2025 07:59:29 GMT, Shaojin Wen <[email protected]> wrote:
>> Shaojin Wen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> remove JLA
>
> I wanted to modify the DecimalDigits.appendQuad method as follows, but this
> caused `MergeStore` to not work.
>
> public static void appendQuad(StringBuilder buf, int v) {
> // The & 0x7f operation keeps the index within the safe range [0,
> 127] for the DIGITS array,
> // which allows the JIT compiler to eliminate array bounds checks for
> performance.
> int packed = DIGITS[(v / 100) & 0x7f] | (DIGITS[(v % 100) & 0x7f] <<
> 16);
> // The temporary String and byte[] objects created here are typically
> eliminated
> // by the JVM's escape analysis and scalar replacement optimizations
> during
> // runtime compilation, avoiding actual heap allocations in optimized
> code.
> buf.append(
> JLA.uncheckedNewStringWithLatin1Bytes(
> new byte[] {(byte) packed, (byte) (packed >>
> 8),
> (byte) (packed >> 16), (byte) (packed >>
> 24)}));
> }
>
>
> The output is as follows:
>
> [TraceMergeStores] MergePrimitiveStores::run: 868 StoreB === 887 813 861
> 145 [[ 872 ]] @byte[int:>=0]
> (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=7;
> Memory: @byte[int:>=0]
> (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=7;
> [TraceMergeStores] expect no use: None
> [TraceMergeStores] expect def: None
> [TraceMergeStores] MergePrimitiveStores::run: 848 StoreB === 888 813 840
> 81 [[ 853 ]] @byte[int:>=0]
> (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=7;
> Memory: @byte[int:>=0]
> (java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=7;
> [TraceMergeStores] expect no use: None
> [TraceMergeStores] expect def: None
> [TraceMergeStores] MergePrimitiveStores::run: 559 StoreB === 548 543 351
> 352 [[ 562 ]] @java/lang/AbstractStringBuilder
> (java/lang/CharSequence,java/lang/Appendable)+16 *, name=coder, idx=13;
> Memory: @java/lang/StringBuilder
> (java/io/Serializable,java/lang/Comparable,java/lang/CharSequence,java/lang/Appendable):NotNull:exact+16
> *, name=coder, idx=13; !jvms: AbstractStringBuilder::append @ bci:78 (line
> 651) StringBuilder::append @ bci:2 (line 179) DecimalDigits::appendQuad @
> bci:68 (line 496)
> [TraceMergeStores] expect no use: None
> [TraceMergeStores] expect def: None
> [TraceMergeStores] MergePrimitiveStores::run: 739 StoreI === 879 813 354
> 456 [[ 17 ]] @java/lang/AbstractStringBuilder
> (java/lang/CharSequence,java/lang/Appendable)+12 *, name=count, idx=14;
> Memory: @java/lang/StringBuilder (java/io/Serializ...
> @wenshao I see. I suspect that your `packed` is deconstructed into byte parts
> by C2 optimizations.
>
> What happens if you route the `packed` value through some non-inlined method?
>
> ```
> int packed = DIGITS[(v / 100) & 0x7f] | (DIGITS[(v % 100) & 0x7f] <<
> 16);
> packed = dontinline(packed); // prevents optimizations
> // The temporary String and byte[] objects created here are typically
> eliminated
> // by the JVM's escape analysis and scalar replacement optimizations
> during
> // runtime compilation, avoiding actual heap allocations in optimized
> code.
> buf.append(
> JLA.uncheckedNewStringWithLatin1Bytes(
> new byte[] {(byte) packed, (byte) (packed >>
> 8),
> (byte) (packed >> 16), (byte) (packed >>
> 24)}));
> ```
>
> The issue is probably that `packed >> 16)` in the lower part sees that it
> only requires the values from `DIGITS[(v % 100) & 0x7f] << 16`, and just
> redirects things, and folds away the shifts. C2 does a lot of smart things
> like that.
>
> Feel free to debug this myself, and look at the C2 IR. I unfortunately have
> no time to dig deeper here at this time.
If `packed` is a constant, then `MergeStore` works here, so your guess is
correct.
Let's leave it at that for now. Two `StoreC` instances are good enough; there's
no need to try merging them into `StoreI`.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/26911#issuecomment-3532952110
