RFR(S): Implementation of aarch64 interpreter matrix barrier
Zhengyu Gu
zgu at redhat.com
Wed Jun 21 13:51:23 UTC 2017
Thanks for the review, Andrew and Aleksey,
I updated patch according to your comments:
Webrev:
http://cr.openjdk.java.net/~zgu/shenandoah/aarch64_int_matrix_barrier/webrev.01/
Thanks,
-Zhengyu
On 06/21/2017 05:41 AM, Andrew Haley wrote:
> On 21/06/17 09:14, Aleksey Shipilev wrote:
>> On 06/21/2017 01:10 AM, Zhengyu Gu wrote:
>>> Interpreter's matrix barrier is missing in aarch64 port.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~zgu/shenandoah/aarch64_int_matrix_barrier/webrev.00/index.html
>>
>> Looks okay to me.
>>
>> You can move this block inside Shenandoah branch:
>>
>> 180 // For Shenandoah, make sure we only store refs into to-space.
>> 181 oopDesc::bs()->interpreter_read_barrier(_masm, val);
>>
>> I would probably write the checks explicitly too, like this:
>>
>> if (UseG1GC) {
>> ...
>> } else if (UseShenandoahGC) {
>> ...
>> } else {
>> ShouldNotReachHere();
>> }
>
> +void MacroAssembler::shenandoah_write_barrier_post(Register store_addr,
> + Register new_val,
> + Register thread,
> + Register tmp,
> + Register tmp2) {
> + assert(thread == rthread, "must be");
> + assert(UseShenandoahGC, "expect Shenandoah GC");
> +
> + if (! UseShenandoahMatrix) {
> + // No need for that barrier if not using matrix.
> + return;
> + }
> +
> + assert_different_registers(store_addr, new_val, thread, tmp, tmp2, rscratch2);
> +
> + Label done;
> + cbz(new_val, done);
> +
> + ShenandoahConnectionMatrix* matrix = ShenandoahHeap::heap()->connection_matrix();
> + address matrix_addr = matrix->matrix_addr();
> + movptr(rscratch2, (intptr_t)ShenandoahHeap::heap()->base());
>
> If an address isn't going to be relocated it makes more sense to use
> adrp, not movptr, here:
>
> unsigned long offset;
> adrp(rscratch2, ExternalAddress(ShenandoahHeap::heap()->base(), offset);
> lea(rscratch2, Address(rscratch2, offset));
>
> The heap base should be page aligned, so the lea won't generate any code.
>
> + mov(tmp, new_val);
> + sub(tmp, tmp, rscratch2);
>
> sub(tmp, new_val, rscratch2);
>
> + lsr(tmp, tmp, ShenandoahHeapRegion::region_size_shift_jint());
> +
> + mov(tmp2, store_addr);
> + sub(tmp2, tmp2, rscratch2);
>
> sub(tmp2, store_addr, rscratch2)
>
> + lsr(tmp2, tmp2, ShenandoahHeapRegion::region_size_shift_jint());
> +
> + movptr(rscratch2, (intptr_t)matrix->stride_jint());
>
> Don't use movptr here: it generates a patchable address, which you
> don't need. Use mov(reg, uint64_t) .
>
> + madd(tmp, tmp, rscratch2, tmp2);
> +
> + movptr(rscratch2, (intptr_t)matrix_addr);
>
> Use mov(dst, address) .
>
> + Address loc(rscratch2, tmp);
> +
> + ldrb(tmp2, loc);
> + cbnz(tmp2, done);
> + mov(tmp2, 1);
> + strb(tmp2, loc);
> + bind(done);
> +}
> +
>
More information about the shenandoah-dev
mailing list