[aarch64-port-dev ] RFR 8233339: Shenandoah: Centralize load barrier decisions into ShenandoahBarrierSet
Zhengyu Gu
zgu at redhat.com
Thu Nov 7 19:01:42 UTC 2019
>
> Filed: https://bugs.openjdk.java.net/browse/JDK-8233401
Rebased on top of JDK-8233401
Webrev: http://cr.openjdk.java.net/~zgu/JDK-8233339/webrev.02/index.html
Thanks,
-Zhengyu
>
> Matter of fact, I would like to hold off this code review, till reactor
> is done.
>
> Thanks,
>
> -Zhengyu
>
>>
>> *) shenandoahBarrierSetAssembler_x86.cpp, I believe it would be more
>> straightforward to save
>> branching on local variable "need_load_reference_barrier" by spelling
>> out the "disabled" path
>> directly (in fact, I think you are almost there in
>> shenandoahBarrierSetC1.cpp!):
>>
>> if (!ShenandoahBarrierSet::need_load_reference_barrier(decorators,
>> type)) {
>> BarrierSetAssembler::load_at(masm, decorators, type, dst, src,
>> tmp1, tmp_thread);
>> return;
>> }
>>
>> ... code that assumes need_load_reference_barrier = true follows ...
>>
>> Register result_dst = dst;
>> bool use_tmp1_for_dst = false;
>>
>> *) shenandoahBarrierSetC1.cpp: local variable
>> "need_load_reference_barrier" is not needed, there is
>> only a single use
>>
>> *) shenandoahBarrierSetC2.cpp: this block should go all the way up:
>>
>> 557 if
>> (!ShenandoahBarrierSet::need_load_reference_barrier(decorators, type)) {
>> 558 return load;
>> 559 }
>>
>> *) shenandoahBarrierSet.cpp: this is just "return
>> is_reference_type(type)". Saves some inversions.
>>
>> 78 if (!is_reference_type(type)) return false;
>> 79 return true;
>>
>> *) shenandoahBarrierSet.cpp: should be "Should be subset of LRB":
>>
>> 83 assert(need_load_reference_barrier(decorators, type), "Why
>> ask?");
>>
>> *) shenandoahBarrierSet.cpp: seems like this assert is subsumed by the
>> previous one?
>>
>> 84 assert(is_reference_type(type), "Why we here?");
>>
>>
More information about the aarch64-port-dev
mailing list