RFR: 8203353: Fixup inferred decorators in the interpreter

[Roman Kennke]

Am 31.05.2018 um 19:21 schrieb 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.

The if statement will go away with this change:
https://bugs.openjdk.java.net/browse/JDK-8203232

Roman

> 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
>>>
>>
>