RFR: 8331935: Add support for primitive array C1 clone intrinsic in PPC

[Martin Doerr]

On Wed, 15 May 2024 13:50:27 GMT, Varada M <varadam at openjdk.org> wrote:

> https://bugs.openjdk.org/browse/JDK-8302850 port for PPC64
> 
> JMH Benchmark Results
> 
> 
> Before :
> 
> Benchmark                 (size)  Mode  Cnt    Score   Error  Units
> ArrayClone.byteArraycopy       0  avgt   15  114.107 ? 1.337  ns/op
> ArrayClone.byteArraycopy      10  avgt   15  130.492 ? 0.991  ns/op
> ArrayClone.byteArraycopy     100  avgt   15  139.103 ? 1.913  ns/op
> ArrayClone.byteArraycopy    1000  avgt   15  321.688 ? 6.033  ns/op
> ArrayClone.byteClone           0  avgt   15  227.602 ? 3.393  ns/op
> ArrayClone.byteClone          10  avgt   15  237.624 ? 2.996  ns/op
> ArrayClone.byteClone         100  avgt   15  239.219 ? 2.835  ns/op
> 
> ArrayClone.byteClone        1000  avgt   15  355.571 ? 2.946  ns/op
> ArrayClone.intArraycopy        0  avgt   15  113.275 ? 1.099  ns/op
> ArrayClone.intArraycopy       10  avgt   15  129.763 ? 1.458  ns/op
> ArrayClone.intArraycopy      100  avgt   15  213.327 ? 2.524  ns/op
> ArrayClone.intArraycopy     1000  avgt   15  449.650 ? 7.338  ns/op
> ArrayClone.intClone            0  avgt   15  225.682 ? 3.048  ns/op
> ArrayClone.intClone           10  avgt   15  234.532 ? 2.817  ns/op
> ArrayClone.intClone          100  avgt   15  295.934 ? 4.925  ns/op
> ArrayClone.intClone         1000  avgt   15  573.368 ? 5.739  ns/op
> Finished running test 'micro:java.lang.ArrayClone'
> Test report is stored in build/aix-ppc64-server-release/test-results/micro_java_lang_ArrayClone
> 
> ==============================
> Test summary
> ==============================
>    TEST                                              TOTAL  PASS  FAIL ERROR   
>    micro:java.lang.ArrayClone                            1     1     0     0   
> ==============================
> TEST SUCCESS
> 
> Finished building target 'test' in configuration 'aix-ppc64-server-release'
> 
> 
> 
> 
> After:
> 
> Benchmark                 (size)  Mode  Cnt    Score    Error  Units
> ArrayClone.byteArraycopy       0  avgt   15  113.894 ?  0.993  ns/op
> ArrayClone.byteArraycopy      10  avgt   15  131.455 ?  0.956  ns/op
> ArrayClone.byteArraycopy     100  avgt   15  139.145 ?  3.002  ns/op
> ArrayClone.byteArraycopy    1000  avgt   15  315.957 ? 14.591  ns/op
> ArrayClone.byteClone           0  avgt   15   43.753 ?  3.669  ns/op
> ArrayClone.byteClone          10  avgt   15   52.329 ?  1.041  ns/op
> ArrayClone.byteClone         100  avgt   15  127.711 ?  3.938  ns/op
> 
> ArrayClone.byteClone        1000  avgt   15  225.937 ?  1.987  ns/op
> ArrayClone.intArraycopy        0  avgt   15  113.788 ?  0.770  ns/op
> ArrayClone.intArraycopy       10  avgt   1...

I also have a minor cleanup proposal for `LIR_Assembler::emit_arraycopy`:

diff --git a/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp b/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp
index dba662a2212..2424d820177 100644
--- a/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp
+++ b/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp
@@ -1827,18 +1827,17 @@ void LIR_Assembler::emit_arraycopy(LIR_OpArrayCopy* op) {
 
   int flags = op->flags();
   ciArrayKlass* default_type = op->expected_type();
-  BasicType basic_type = default_type != nullptr ? default_type->element_type()->basic_type() : T_ILLEGAL;
+  BasicType basic_type = (default_type != nullptr) ? default_type->element_type()->basic_type() : T_ILLEGAL;
   if (basic_type == T_ARRAY) basic_type = T_OBJECT;
 
   // Set up the arraycopy stub information.
   ArrayCopyStub* stub = op->stub();
-  const int frame_resize = frame::native_abi_reg_args_size - sizeof(frame::java_abi); // C calls need larger frame.
 
   // Always do stub if no type information is available. It's ok if
   // the known type isn't loaded since the code sanity checks
   // in debug mode and the type isn't required when we know the exact type
   // also check that the type is an array type.
-  if (op->expected_type() == nullptr) {
+  if (default_type == nullptr) {
     assert(src->is_nonvolatile() && src_pos->is_nonvolatile() && dst->is_nonvolatile() && dst_pos->is_nonvolatile() &&
            length->is_nonvolatile(), "must preserve");
     address copyfunc_addr = StubRoutines::generic_arraycopy();
@@ -1873,7 +1872,7 @@ void LIR_Assembler::emit_arraycopy(LIR_OpArrayCopy* op) {
     return;
   }
 
-  assert(default_type != nullptr && default_type->is_array_klass(), "must be true at this point");
+  assert(default_type != nullptr && default_type->is_array_klass() && default_type->is_loaded(), "must be true at this point");
   Label cont, slow, copyfunc;
 
   bool simple_check_flag_set = flags & (LIR_OpArrayCopy::src_null_check |

Would be nice to have.

src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp line 903:

> 901:     info = state_for(x, x->state());
> 902:   }
> 903: 

This code seems to be integrated at the wrong place. Other platforms have it in `LIRGenerator::do_NewTypeArray`.

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

PR Comment: https://git.openjdk.org/jdk/pull/19250#issuecomment-2117816097
PR Review Comment: https://git.openjdk.org/jdk/pull/19250#discussion_r1605160938