[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