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

[varada1110]

trafficstars

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   15  131.980 ?  2.102  ns/op
ArrayClone.intArraycopy      100  avgt   15  213.745 ?  2.615  ns/op
ArrayClone.intArraycopy     1000  avgt   15  460.820 ?  7.106  ns/op
ArrayClone.intClone            0  avgt   15   42.074 ?  0.547  ns/op
ArrayClone.intClone           10  avgt   15   80.125 ?  1.735  ns/op
ArrayClone.intClone          100  avgt   15  207.313 ?  2.717  ns/op
ArrayClone.intClone         1000  avgt   15  326.546 ?  5.671  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

Hotspot compiler tests results :

==============================
Test summary
==============================
   TEST                                              TOTAL  PASS  FAIL ERROR   
>> jtreg:test/hotspot/jtreg:hotspot_compiler          1170  1168     2     0 <<
==============================
TEST FAILURE

2 test failures shown here is not related to code change. It is present without this changes

Reported Issue : JDK-8331935

---

Progress

• [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue

Issue

• JDK-8331935: Add support for primitive array C1 clone intrinsic in PPC (Enhancement - P4)

Reviewers

• Amit Kumar (@offamitkumar - Committer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19250/head:pull/19250
$ git checkout pull/19250

Update a local copy of the PR:
$ git checkout pull/19250
$ git pull https://git.openjdk.org/jdk.git pull/19250/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19250

View PR using the GUI difftool:
$ git pr show -t 19250

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19250.diff

Webrev

Link to Webrev Comment

[bridgekeeper[bot]]

:wave: Welcome back varadam! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk[bot]]

@varada1110 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

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

Reviewed-by: mdoerr, amitkumar

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 91 new commits pushed to the master branch:

• f7862bd6b9994814c6dfd43d471122408601f288: 8331311: C2: Big Endian Port of 8318446: optimize stores into primitive arrays by combining values into larger store
• b4beda21b487886b022e04766e140e6d1df1038a: 8332537: C2: High memory usage reported for compiler/loopopts/superword/TestAlignVectorFuzzer.java
• e5383d710c0727181a2f0b569a881de2492e3683: 8333713: C2 SuperWord: cleanup in vectornode.cpp/hpp
• 944aeb81b16e3e7a3019cafdefe67b797fa6be96: 8325155: C2 SuperWord: remove alignment boundaries
• d8af58941b5dedb9774c0971895c4924e57ac28b: 8026127: Deflater/Inflater documentation incomplete/misleading
• 6238bc8da2abe7a1f0cdd98c0af01e9ba1869ec3: 8333456: CompactNumberFormat integer parsing fails when string has no suffix
• 2a37764e7428d579a3080e62681f1c9c9f816c1e: 8333743: Change .jcheck/conf branches property to match valid branches
• 75dc2f8518d0adea30f7065d6732b807c0220756: 8330182: Start of release updates for JDK 24
• 054362abe040938b87eb1a1cab8a0a94540e0667: 8332550: [macos] Voice Over: java.awt.IllegalComponentStateException: component must be showing on the screen to determine its location
• 9b436d048ec92f74ec6812ae20fde21751927d4b: 8333674: Disable CollectorPolicy.young_min_ergo_vm for PPC64
• ... and 81 more: https://git.openjdk.org/jdk/compare/75220da26f647c6f3dabc05cea81cefaf3a1e195...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@TheRealMDoerr, @offamitkumar) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[openjdk[bot]]

@varada1110 The following label will be automatically applied to this pull request:

• hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[TheRealMDoerr]

The test failures will be fixed by https://github.com/openjdk/jdk/pull/19218. Unrelated.

[varada1110]

I got crashes when testing on linux ppc64le and noticed that we need one more adaptation to handle stub == nullptr. I suggest the following addition:

diff --git a/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp b/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp
index b6d9200b261..dba662a2212 100644
--- a/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp
+++ b/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp
@@ -1968,7 +1968,11 @@ void LIR_Assembler::emit_arraycopy(LIR_OpArrayCopy* op) {
   int shift = shift_amount(basic_type);
 
   if (!(flags & LIR_OpArrayCopy::type_check)) {
-    __ b(cont);
+    if (stub != nullptr) {
+      __ b(cont);
+      __ bind(slow);
+      __ b(*stub->entry());
+    }
   } else {
     // We don't know the array types are compatible.
     if (basic_type != T_OBJECT) {
@@ -2089,9 +2093,9 @@ void LIR_Assembler::emit_arraycopy(LIR_OpArrayCopy* op) {
         __ add(dst_pos, tmp, dst_pos);
       }
     }
+    __ bind(slow);
+    __ b(*stub->entry());
   }
-  __ bind(slow);
-  __ b(*stub->entry());
   __ bind(cont);
 
 #ifdef ASSERT

Hi @TheRealMDoerr , I have applied the suggested changes and I have fixed the indentation fixes. Testing is also done. Thank you

[mlbridge[bot]]

Webrevs

• 04: Full (b339ecb6)
• 03: Full (8e4e05ab)
• 02: Full - Incremental (28013450)
• 01: Full - Incremental (0aa5b21c)
• 00: Full (f484bbee)

[varada1110]

Thanks @TheRealMDoerr Hi @offamitkumar, Could you please review the code?

[varada1110]

I got test failures on AIX which need investigation: compiler/c2/Test6910605_2.java assert(oopDesc::is_oop(s)) failed: JVM_ArrayCopy: src not an oop

Hi @TheRealMDoerr , this test failure was not showing for me. I retested with 'JAVA_OPTIONS=-XX:TieredStopAtLevel=1' and the test is passing

==============================
Test summary
==============================
   TEST                                              TOTAL  PASS  FAIL ERROR   
   jtreg:test/hotspot/jtreg/compiler/c2/Test6910605_2.java
                                                         1     1     0     0   
==============================
TEST SUCCESS

[TheRealMDoerr]

I can reproduce it on linux with the fastdebug build.

[varada1110]

I can reproduce it on linux with the fastdebug build.

Yes. The test failing with fastdebug build

#
# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error (/home/hotspot/openjdk/jdk-varada/src/hotspot/share/prims/jvm.cpp:301), pid=27263472, tid=4884
#  assert(oopDesc::is_oop(s)) failed: JVM_ArrayCopy: src not an oop
#
# JRE version: OpenJDK Runtime Environment (23.0) (fastdebug build 23-internal-adhoc.hotspot.jdk-varada)
# Java VM: OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.hotspot.jdk-varada, mixed mode, emulated-client, tiered, compressed oops, compressed class ptrs, g1 gc, aix-ppc64)
# No core dump will be written. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again
#
# An error report file with more information is saved as:
# /home/hotspot/openjdk/jdk-varada/build/aix-ppc64-server-fastdebug/test-support/jtreg_test_hotspot_jtreg_compiler_c2_Test6910605_2_java/scratch/0/hs_err_pid27263472.log
[0.762s][warning][os] Loading hsdis library failed
#
# If you would like to submit a bug report, please visit:
#   https://bugreport.java.com/bugreport/crash.jsp
#```

[TheRealMDoerr]

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.

[varada1110]

Thank you @TheRealMDoerr @offamitkumar . I am running the tests: hotspot_compiler, hotspot_gc, hotspot_serviceability and hotspot_runtime for tier1, tier2 and tier3 with fastdebug, slowdebug and release. I will update the results.

[offamitkumar]

with fastdebug, slowdebug and release.

I think with fastdebug is sufficient.

[TheRealMDoerr]

I've put it again into our nightly tests and haven't seen any errors which may have been caused by this PR. There are currently some unrelated errors. So, I think it's good to go.

[shipilev]

There is currently a regression in the original code, JDK-8332670, which may explain some instability on PPC.

[TheRealMDoerr]

Thanks for the hint! We should wait for that one to be fixed.

[varada1110]

Thank you @TheRealMDoerr @offamitkumar . I am running the tests: hotspot_compiler, hotspot_gc, hotspot_serviceability and hotspot_runtime for tier1, tier2 and tier3 with fastdebug, slowdebug and release. I will update the results.

Completed the testing for fastdebug. There are few unrelated test failures

[TheRealMDoerr]

https://github.com/openjdk/jdk/pull/19538 is integrated, so we can ship this one, too.

[varada1110]

/integrate

[openjdk[bot]]

@varada1110 Your change (at version b339ecb68f51f0a8dded9041c0b2ace5d1d6911c) is now ready to be sponsored by a Committer.

[offamitkumar]

/sponsor

[openjdk[bot]]

Going to push as commit 6968770b1e918c74fc009e3562a827bb4acbe2d7. Since your change was applied there have been 94 commits pushed to the master branch:

• a2030fff9833aba40e8c7c177151a30a0812a250: 8332516: Serial: Always sample promoted bytes to avoid getting stuck in Full GCs
• bf7f1c41cc2a2b98775301bc377a4c6e1340a736: 8333211: NMT Reports: replace manual indentation handling with auto indent
• 8ffc35d117846a7a2aa08afed662273d2f887770: 8333724: Problem list security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#teliasonerarootcav1
• f7862bd6b9994814c6dfd43d471122408601f288: 8331311: C2: Big Endian Port of 8318446: optimize stores into primitive arrays by combining values into larger store
• b4beda21b487886b022e04766e140e6d1df1038a: 8332537: C2: High memory usage reported for compiler/loopopts/superword/TestAlignVectorFuzzer.java
• e5383d710c0727181a2f0b569a881de2492e3683: 8333713: C2 SuperWord: cleanup in vectornode.cpp/hpp
• 944aeb81b16e3e7a3019cafdefe67b797fa6be96: 8325155: C2 SuperWord: remove alignment boundaries
• d8af58941b5dedb9774c0971895c4924e57ac28b: 8026127: Deflater/Inflater documentation incomplete/misleading
• 6238bc8da2abe7a1f0cdd98c0af01e9ba1869ec3: 8333456: CompactNumberFormat integer parsing fails when string has no suffix
• 2a37764e7428d579a3080e62681f1c9c9f816c1e: 8333743: Change .jcheck/conf branches property to match valid branches
• ... and 84 more: https://git.openjdk.org/jdk/compare/75220da26f647c6f3dabc05cea81cefaf3a1e195...master

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@offamitkumar @varada1110 Pushed as commit 6968770b1e918c74fc009e3562a827bb4acbe2d7.

:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.