RFR: JDK-8231583: Shenandoah: Fix register clash in SBSA::resolve_forwarding_pointer() borrowing

Roman Kennke rkennke at redhat.com
Mon Sep 30 14:57:25 UTC 2019


Hi Aleksey,

>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8231583
>> Webrev:
>> http://cr.openjdk.java.net/~rkennke/JDK-8231583/webrev.00/
> 
> Awww.
> 
> Please change the synopsis to reflect what is being done, not the symptom of it. Suggestion:
>   Shenandoah: register clash in SBSA::resolve_forwarding_pointer borrowing

Right. Changed to:
Shenandoah: Fix register clash in SBSA::resolve_forwarding_pointer()
borrowing

> I believe we are better off just fixing the borrowing scheme, like (not heavily tested):
> 
> diff -r 6902d5acd5e8 src/hotspot/cpu/aarch64/gc/shenandoah/shenandoahBarrierSetAssembler_aarch64.cpp
> --- a/src/hotspot/cpu/aarch64/gc/shenandoah/shenandoahBarrierSetAssembler_aarch64.cpp   Mon Sep 30
> 10:19:03 2019 +0200
> +++ b/src/hotspot/cpu/aarch64/gc/shenandoah/shenandoahBarrierSetAssembler_aarch64.cpp   Mon Sep 30
> 15:30:52 2019 +0200
> @@ -212,7 +212,12 @@
>      // No free registers available. Make one useful.
>      tmp = rscratch1;
> +    if (tmp == dst) {
> +      tmp = rscratch2;
> +    }
>      __ push(RegSet::of(tmp), sp);
>    }
> 
> +  assert_different_registers(tmp, dst);
> +
>    Label done;
>    __ ldr(tmp, Address(dst, oopDesc::mark_offset_in_bytes()));
> diff -r 6902d5acd5e8 src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetAssembler_x86.cpp
> --- a/src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetAssembler_x86.cpp   Mon Sep 30 10:19:03
> 2019 +0200
> +++ b/src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetAssembler_x86.cpp   Mon Sep 30 15:30:52
> 2019 +0200
> @@ -272,7 +272,12 @@
>      // No free registers available. Make one useful.
>      tmp = LP64_ONLY(rscratch1) NOT_LP64(rdx);
> +    if (tmp == dst) {
> +      tmp = LP64_ONLY(rscratch2) NOT_LP64(rcx);
> +    }
>      __ push(tmp);
>    }
> 
> +  assert_different_registers(dst, tmp);
> +
>    Label done;
>    __ movptr(tmp, Address(dst, oopDesc::mark_offset_in_bytes()));
> 
> 

Yeah.
I took it a little further and removed the tmp argument to
resolve_forwarding_pointer() altogether because we have no caller that
passes anything interesting to it.

http://cr.openjdk.java.net/~rkennke/JDK-8231583/webrev.01/

I believe aarch64 should be good (and should actually not require
borrowing) because rscratch1 is not used by the register allocators there.

Ok now?

Roman




More information about the hotspot-gc-dev mailing list