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