[aarch64-port-dev ] RFR/RFC: Shenandoah integration 2021-03-11

[Andrew Hughes]

On 18:33 Thu 11 Mar     , Aleksey Shipilev wrote:
> On 3/11/21 6:22 PM, Andrew Hughes wrote:
> > > There is no merge changeset, as shenandoah/jdk8 forest is clean downstream
> > > of integration repository. I am tagging the whole thing as
> > > aarch64-shenandoah-jdk8u292-b05-shenandoah-merge-2021-03-11.
> > 
> > Sounds good. The webrev does suggest there are merge revisions though:
> > 
> > https://cr.openjdk.java.net/~shade/shenandoah/merges/aarch64-port-8u-20210311/webrev.01/
> > 
> > e.g. "rev 12213 : Merge"
> 
> Yes, these are merges during the pickups _from_ aarch64-port/jdk8u-shenandoah to shenandoah/jdk8.
>

Ok, thought so.

> > > Testing: hotspot_gc_shenandoah; it also runs pretty stably in our CIs, and
> > > we have no user bugreports from our 8u nightlies
> > 
> > Was it tested without INCLUDE_ALL_GCS defined?
> 
> Yes, our (well, my) CI implicitly tests this with when building Minimal VM.
> It does it for all repositories, including aarch64-port/jdk8u-shenandoah and
> shenandoah/jdk8, so neither side is broken at the moment :)
>

Good to hear :)

> > My only concern is the backporting of 8202976: "Add C1 lea patching support for x86"
> 
> Yeah, alas we needed it for Shenandoah to do self-fixing barriers.
> 
> > It looks like the changes will have no effect when Shenandoah is disabled at runtime,
> > though the code changes differ between using INCLUDE_ALL_GCS as well in
> > c1_LIRAssembler_aarch64.cpp but not in c1_LIRAssembler_x86.cpp (difficult to do
> > so there with the structure of the code).
> > 
> > At the very least, this will produce some unused variable warnings, but that is true
> > of JDK-8202976 in 11u too.
> 
> It should not produce unused warnings, as UseShenandoahGC would always be available.
>

I was thinking of AArch64 & sparc:

--- old/src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp	2021-03-11 15:54:23.162720736 +0100
+++ new/src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp	2021-03-11 15:54:23.086718776 +0100
@@ -2908,7 +2908,14 @@
 }
 
 
-void LIR_Assembler::leal(LIR_Opr addr, LIR_Opr dest) {
+void LIR_Assembler::leal(LIR_Opr addr, LIR_Opr dest, LIR_PatchCode patch_code, CodeEmitInfo* info) {
+#if INCLUDE_ALL_GCS
+  if (UseShenandoahGC && patch_code != lir_patch_none) {
+    deoptimize_trap(info);
+    return;
+  }
+#endif
+
   __ lea(dest->as_register_lo(), as_Address(addr->as_address_ptr()));
 }

If INCLUDE_ALL_GCS is not set, then nothing will use patch_code and info.

--- old/src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp	2021-03-11 15:54:26.550808044 +0100
+++ new/src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp	2021-03-11 15:54:26.478806188 +0100
@@ -3486,7 +3486,7 @@
 }
 
 
-void LIR_Assembler::leal(LIR_Opr addr_opr, LIR_Opr dest) {
+void LIR_Assembler::leal(LIR_Opr addr_opr, LIR_Opr dest, LIR_PatchCode patch_code, CodeEmitInfo* info) {

This is the same as 11u. Again, info & patch_code will never be used.

> IIRC, Andrew Haley lifted the prior policy that every Shenandoah block
> should always be protected by INCLUDE_ALL_GCS. We now "only" use it to
> protect the code blocks that touch Shenandoah symbols, which might not be
> available in some configs. c1_LIRAssembler_x86.cpp is not such a block.
>

Yeah, as I say, it would be difficult to do otherwise, without having to repeat
the same non-Shenandoah code for the !UseShenandoahGC + INCLUDE_ALL_GCS case and
the no INCLUDE_ALL_GCS case.

> Anything else you want answered / done?
>

I'm happy for this to go in.

> -- 
> Thanks,
> -Aleksey
> 

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
OpenJDK Package Owner
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222