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

[Dean Long]

On Fri, 5 Apr 2024 02:16:55 GMT, Dean Long <dlong at openjdk.org> wrote:

>> 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.
>
> I think we could eventually relax the requirement that receiver_klass be loaded, at least for object arrays, but for simplicity my patch will follow the existing behavior.

> @dean-long I tried your patch and my test worked fine with it. I had one doubt about the following:
> 
> ```
>     Value recv = apop();
>     apush(recv);
> ```
> 
> Wouldn't you prefer to just peek the top of the stack rather than pop it to push it again? I would expect the former to be cheaper to do?

Sure, feel free to improve it.

> > My patch still needs some work.
> 
> What else does it need?

I fixed the issue I had, so it should be good to test out now.  Thanks.

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

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