RFR: 8302850: Implement C1 clone intrinsic that reuses arraycopy code for primitive arrays [v7]

[Dean Long]

On Wed, 20 Mar 2024 09:04:36 GMT, Galder Zamarreño <galder at openjdk.org> wrote:

>> Adding C1 intrinsic for primitive array clone invocations for aarch64 and x86 architectures.
>> 
>> The intrinsic includes a change to avoid zeroing the newly allocated array because its contents are copied over within the same intrinsic with arraycopy. This means that the performance of primitive array clone exceeds that of primitive array copy. As an example, here are the microbenchmark results on darwin/aarch64:
>> 
>> 
>> $ make test TEST="micro:java.lang.ArrayClone" MICRO="JAVA_OPTIONS=-XX:TieredStopAtLevel=1"
>> Benchmark                 (size)  Mode  Cnt    Score    Error  Units
>> ArrayClone.byteArraycopy       0  avgt   15    3.476 ?  0.018  ns/op
>> ArrayClone.byteArraycopy      10  avgt   15    3.740 ?  0.017  ns/op
>> ArrayClone.byteArraycopy     100  avgt   15    7.124 ?  0.010  ns/op
>> ArrayClone.byteArraycopy    1000  avgt   15   39.301 ?  0.106  ns/op
>> ArrayClone.byteClone           0  avgt   15    3.478 ?  0.008  ns/op
>> ArrayClone.byteClone          10  avgt   15    3.562 ?  0.007  ns/op
>> ArrayClone.byteClone         100  avgt   15    5.888 ?  0.206  ns/op
>> ArrayClone.byteClone        1000  avgt   15   25.762 ?  0.203  ns/op
>> ArrayClone.intArraycopy        0  avgt   15    3.199 ?  0.016  ns/op
>> ArrayClone.intArraycopy       10  avgt   15    4.521 ?  0.008  ns/op
>> ArrayClone.intArraycopy      100  avgt   15   17.429 ?  0.039  ns/op
>> ArrayClone.intArraycopy     1000  avgt   15  178.432 ?  0.777  ns/op
>> ArrayClone.intClone            0  avgt   15    3.406 ?  0.016  ns/op
>> ArrayClone.intClone           10  avgt   15    4.272 ?  0.006  ns/op
>> ArrayClone.intClone          100  avgt   15   13.110 ?  0.122  ns/op
>> ArrayClone.intClone         1000  avgt   15  113.196 ? 13.400  ns/op
>> 
>> 
>> It also includes an optimization to avoid instantiating the array copy stub in scenarios like this.
>> 
>> I run hotspot compiler tests successfully limiting them to C1 compilation darwin/aarch64, linux/x86_64 and linux/686. E.g.
>> 
>> 
>> $ make test TEST="hotspot_compiler" JTREG="JAVA_OPTIONS=-XX:TieredStopAtLevel=1"
>> ...
>>    TEST                                              TOTAL  PASS  FAIL ERROR
>>    jtreg:test/hotspot/jtreg:hotspot_compiler          1234  1234     0     0
>> 
>> 
>> One question I had is what to do about non-primitive object arrays, see my [question](https://bugs.openjdk.org/browse/JDK-8302850?focusedId=14634879&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14634879) on the issue. @cl4es any thoughts?
>> 
>>...
>
> Galder Zamarreño has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits:
> 
>  - Merge branch 'master' into topic.0131.c1-array-clone
>  - Merge branch 'master' into topic.0131.c1-array-clone
>  - Reserve necessary frame map space for clone use cases
>  - 8302850: C1 primitive array clone intrinsic in graph
>    
>    * Combine array length, new type array and arraycopy for clone in c1 graph.
>    * Add OmitCheckFlags to skip arraycopy checks.
>    * Instantiate ArrayCopyStub only if necessary.
>    * Avoid zeroing newly created arrays for clone.
>    * Add array null after c1 clone compilation test.
>    * Pass force reexecute to intrinsic via value stack.
>    This is needed to be able to deoptimize correctly this intrinsic.
>    * When new type array or array copy are used for the clone intrinsic,
>    their state needs to be based on the state before for deoptimization
>    to work as expected.
>  - Revert "8302850: Primitive array copy C1 intrinsic for aarch64 and x86"
>    
>    This reverts commit fe5d916724614391a685bbef58ea939c84197d07.
>  - 8302850: Link code emit infos for null check and alloc array
>  - 8302850: Null check array before getting its length
>    
>    * Added a jtreg test to verify the null check works.
>    Without the fix this test fails with a SEGV crash.
>  - 8302850: Force reexecuting clone in case of a deoptimization
>    
>    * Copy state including locals for clone
>    so that reexecution works as expected.
>  - 8302850: Avoid instantiating array copy stub for clone use cases
>  - 8302850: Primitive array copy C1 intrinsic for aarch64 and x86
>    
>    * Clone calls that involve Phi nodes are not supported.
>    * Add unimplemented stubs for other platforms.

My suggested cleanup is here:
https://github.com/dean-long/jdk/tree/pr/17667
Also, you'll need a 2nd review.

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

PR Comment: https://git.openjdk.org/jdk/pull/17667#issuecomment-2038391526