[jdk23] RFR: 8335390: C2 MergeStores: wrong result with Unsafe

[Emanuel Peter]

On Thu, 25 Jul 2024 01:16:46 GMT, Shaojin Wen <duke at openjdk.org> wrote:

>> Thanks @TobiHartmann for the review.
>
> @eme64 Unsafe.putByte MergeStore of master branch doesn't work
> 
> import sun.misc.Unsafe;
> 
> public class PutBytesTest {
>     static final Unsafe UNSAFE = JDKUtils.UNSAFE;
> 
>     // MergeStore not work
>     static void putCharsAt(byte[] val, int index, int c1, int c2, int c3, int c4) {
>         putByte(val, index    , (byte)(c1));
>         putByte(val, index + 1, (byte)(c2));
>         putByte(val, index + 2, (byte)(c3));
>         putByte(val, index + 3, (byte)(c4));
>     }
> 
>     // MergeStore work
>     static void putCharsAtArrayStore(byte[] val, int index, int c1, int c2, int c3, int c4) {
>         val[index] = (byte) c1;
>         val[index + 1] = (byte) c2;
>         val[index + 2] = (byte) c3;
>         val[index + 3] = (byte) c4;
>     }
> 
>     static void putByte(byte[] bytes, int index, byte c) {
>         UNSAFE.putByte(bytes, Unsafe.ARRAY_BYTE_BASE_OFFSET + index, c);
>     }
> 
>     static void putNull(byte[] bytes, int index) {
>         putCharsAt(bytes, index, 'n', 'u', 'l', 'l');
>     }
> 
>     public static void main(String[] args) {
>         for (int i = 0; i < 5; i++) {
>             testNull();
>         }
>         System.out.println("done");
>     }
> 
>     private static void testNull() {
>         byte[] bytes = new byte[4096];
>         for (int i = 0; i < 1000; i++) {
>             int index = 0;
>             for (int j = 0; j < 1024; j++) {
>                 putNull(bytes, index);
>                 index += 4;
>             }
>         }
>     }
> }

@wenshao yes, that is the effect of this PR here. I'm working on a corresponding improvement already for a few weeks. It is a lot of work, because I need to get the testing right, to ensure all possible shapes are handled correctly and do not lead to wrong results.

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

PR Comment: https://git.openjdk.org/jdk/pull/19985#issuecomment-2249540754