RFR: JDK-8202714: Create a MacroAssembler::access_load/store_at wrapper for AArch64

[Roman Kennke]

Am 15.05.2018 um 16:35 schrieb Andrew Haley:
> On 05/15/2018 02:40 PM, Roman Kennke wrote:
>> Am 15.05.2018 um 15:07 schrieb Andrew Haley:
>>>
>>>> On 14 May 2018 at 13:02, Roman Kennke <rkennke at redhat.com> wrote:
>>>>> This reshuffles AArch64 code around BarrierSetAssembler calls follow the
>>>>> equivalent code in x86:
>>>>>
>>>>> http://cr.openjdk.java.net/~rkennke/JDK-8202714/webrev.00/
>>>>>
>>>>> No regressions in hotspot/tier1
>>>>>
>>>>> Can I please get a review?
>>>
>>> Looks OK.  This use of an explict base class override in MacroAssembler
>>> is rather nasty:
>>>
>>> 3990 void MacroAssembler::access_store_at(BasicType type, DecoratorSet decorators,
>>> 3991                                      Address dst, Register src,
>>> 3992                                      Register tmp1, Register thread_tmp) {
>>> 3993   BarrierSetAssembler *bs = BarrierSet::barrier_set()->barrier_set_assembler();
>>> 3994   bool as_raw = (decorators & AS_RAW) != 0;
>>> 3995   if (as_raw) {
>>> 3996     bs->BarrierSetAssembler::store_at(this, decorators, type, dst, src, tmp1, thread_tmp);
>>> 3997   } else {
>>> 3998     bs->store_at(this, decorators, type, dst, src, tmp1, thread_tmp);
>>> 3999   }
>>> 4000 }
>>> 4001
>>
>>
>> It just mirrors what is done in x86 and elsewhere. :-)
> Yeah, I'm sure it does, but one class using internal knowledge of another
> class's inheritance hierarchy is an anti-pattern.
> 

Right. I guess we could have all the implementations check for AS_RAW,
and if so, pass the call up the inheritance hierarchy until it hits the
base class BarrierSetAssembler. Sounds like more boilerplate but more
clean. I'd do that under a separate RFE though, the goal of this one was
to align aarch64 with x86 implementation, changing the call chain would
affect all the implementations.

Roman