RFR: 8203353: Fixup inferred decorators in the interpreter

[coleen.phillimore at Oracle.com]

http://cr.openjdk.java.net/~eosterlund/8203353/webrev.00/src/hotspot/cpu/x86/macroAssembler_x86.cpp.frames.html

6334 void MacroAssembler::access_store_at(BasicType type, DecoratorSet decorators, Address dst, Register src,
6335                                      Register tmp1, Register tmp2) {
6336   BarrierSetAssembler* bs = BarrierSet::barrier_set()->barrier_set_assembler();
6337 decorators = AccessInternal::decorator_fixup(decorators);
6338   bool as_raw = (decorators & AS_RAW) != 0;
6339   if (as_raw) {
6340     bs->BarrierSetAssembler::store_at(this, decorators, type, dst, src, tmp1, tmp2);
6341   } else {
6342     bs->store_at(this, decorators, type, dst, src, tmp1, tmp2);
6343   }
6344 }

This is a very strange function.  From the macroAssembler, how are we to 
know that the cases in this if statement are different?  It seems like 
this distinction should be made in BarrierSetAssembler class.   It also 
seems like whatever decorator fixup should also happen inside the 
BarrierSetAssembler, because the placement of these calls to fixup the 
decorators seems unknowable.

thanks,
Coleen

On 5/31/18 10:23 AM, Erik Österlund wrote:
> Hi Roman,
>
> Thanks for the review!
>
> /Erik
>
> On 2018-05-31 15:40, Roman Kennke wrote:
>>> Hi,
>>>
>>> All subsystems that the access API touches, except the interpreter,
>>> infers a set of decorators according to rules specified in
>>> accessDecorators.hpp. These are typically sane defaults for things like
>>> reference strength, memory ordering, as well as inferring that
>>> IN_HEAP_ARRAY is IN_HEAP, and IN_CONCURRENT_ROOT is IN_ROOT. The
>>> interpreter should do that too.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~eosterlund/8203353/webrev.00/
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8203353
>>
>> Hi Erik, this seems reasonable and patch looks good. Thanks!
>> Roman
>>
>