RFR: Add comments in shenandoah_store_check on direct heap field use

Roman Kennke rkennke at redhat.com
Mon Oct 2 11:05:14 UTC 2017


Am 02.10.2017 um 12:55 schrieb Aleksey Shipilev:
> As suggested by aph last week:
>
> ------------- 8< -----------------------------------------------------------------------
>
> $ hg diff
> diff -r 533d1c8a56d2 src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
> --- a/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp	Fri Sep 29 13:46:48 2017 +0000
> +++ b/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp	Mon Oct 02 12:45:25 2017 +0200
> @@ -5651,6 +5651,9 @@
>
>     // During evacuation and evacuation only, we can have the stores of cset-values
>     // to non-cset destinations. Everything else is covered by storeval barriers.
> +  // Poll the heap directly: that would be the least performant, yet more reliable way,
> +  // because it will also capture the errors in thread-local flags that may break the
> +  // write barrier.
>     mov(tmp1, ShenandoahHeap::evacuation_in_progress_addr());
>     ldrw(tmp1, Address(tmp1));
>     cbnzw(tmp1, done);
> diff -r 533d1c8a56d2 src/cpu/x86/vm/macroAssembler_x86.cpp
> --- a/src/cpu/x86/vm/macroAssembler_x86.cpp	Fri Sep 29 13:46:48 2017 +0000
> +++ b/src/cpu/x86/vm/macroAssembler_x86.cpp	Mon Oct 02 12:45:25 2017 +0200
> @@ -6446,6 +6446,9 @@
>
>     // During evacuation and evacuation only, we can have the stores of cset-values
>     // to non-cset destinations. Everything else is covered by storeval barriers.
> +  // Poll the heap directly: that would be the least performant, yet more reliable way,
> +  // because it will also capture the errors in thread-local flags that may break the
> +  // write barrier.
>     movptr(tmp, (intptr_t) ShenandoahHeap::evacuation_in_progress_addr());
>     movbool(tmp, Address(tmp, 0));
>     testbool(tmp);
>
> ------------- 8< -----------------------------------------------------------------------
>
> Turns out, we poll any flags only on this path in the checking code.
>
> Also, I note the inconsistency here: we should use ldrb/cbnz in aarch64, right?

Yes to change. Yes to inconsistency question ;-)

Roman



More information about the shenandoah-dev mailing list