[aarch64-port-dev ] RFR 8233401: Shenandoah: Refactor/cleanup Shenandoah load barrier code

[Zhengyu Gu]

Updated: http://cr.openjdk.java.net/~zgu/JDK-8233401/webrev.01/index.html

Okay now?

Thanks,

-Zhengyu

On 11/4/19 10:35 AM, Roman Kennke wrote:
>>> Please review this refactor of Shenandoah load barrier. The goal is to make the barrier structurally
>>> similar cross interpreter, C1 and C2, improve readability and maintainability.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8233401
>>> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8233401/webrev.00/index.html
>>
>> This is cute patch.
>>
>> *) Typo "non-reference load":
>>
>>   207   // 1: none-reference load, no additional barrier is needed
>>
>> *) The comment style is inconsistent with other places:
>>
>>   537 Node* ShenandoahBarrierSetC2::load_at_resolved(C2Access& access, const Type* val_type) const {
>>   538   // 1: load reference
>>   539   Node* load = BarrierSetC2::load_at_resolved(access, val_type);
>>   540   // For none-reference load, no additional barrier is needed
>>
>> *) In constructions like this, it seems more consistent to introduce the local variable for matching
>> the decorator?
>>
>>   387     // Native barrier is for concurrent root processing
>>   388     if (((decorators & IN_NATIVE) != 0) &&
>>   389         ShenandoahConcurrentRoots::can_do_concurrent_roots()) {
>>
>> Otherwise looks good. Roman needs to take a look as well.
> 
> Yes, otherwise looks good.
> 
> Thanks,
> Roman
> 
>