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 shenandoah-dev
mailing list