RFR: JDK-8209667: Explicit barriers for C1/LIR

[Roman Kennke]

Thanks, Erik!!

Roman

> Hi Roman,
> 
> Looks good now.
> 
> Thanks,
> /Erik
> 
> On 2018-08-22 13:41, Roman Kennke wrote:
>> Uhh, funny. I brought over this change from Shenandoah not realizing
>> that the CodeEmitInfo was not actually used anymore. I am sure we did
>> use it in the past, but this need seems to have disappeared since then.
>> Alright, let's scratch it (hurra, this will eliminate a whole lot of
>> fluff from Shenandoah...):
>>
>> Incremental:
>> http://cr.openjdk.java.net/~rkennke/JDK-8209667/webrev.03.diff/
>> Full:
>> http://cr.openjdk.java.net/~rkennke/JDK-8209667/webrev.03/
>>
>> Good now?
>>
>> Thanks for reviewing!
>> Roman
>>
>>
>>> Hi Roman,
>>>
>>> I think it boils down to the CodeEmitInfo stuff. I looked through again,
>>> and still only ever saw a NULL parameter being passed in from all
>>> callsites. Did I miss a callsite that does pass in something other than
>>> NULL?
>>>
>>> If not, then CodeEmitInfo seems to not be used, and then I think I would
>>> prefer to pass the arguments explicitly and remove that parameter. If it
>>> is used and I missed where from, then I think I prefer baking it in a
>>> context object like you have already done.
>>>
>>> Thanks,
>>> /Erik
>>>
>>> On 2018-08-22 13:07, Roman Kennke wrote:
>>>>>> thanks for reviewing!
>>>>>>
>>>>>>> Looks good in general. A few minor things though:
>>>>>>>
>>>>>>> 1) I think IN_HEAP should be removed from the access_resolve
>>>>>>> callsites
>>>>>>> (because IN_HEAP is for differentiating if an access hits inside or
>>>>>>> outside of the heap, and this is seemingly orthogonal).
>>>>>> Good point. Fixed.
>>>>>>
>>>>>>> 2) I see you used the LIRAccess object to wrap the context of the
>>>>>>> resolution. However, in this case since this is not an access, this
>>>>>>> seems to cause more trouble than it solves. The only parameters that
>>>>>>> apply is the address (which could be represented with a LIROpr),
>>>>>>> and the
>>>>>>> decorators. So I think I would just pass those two parameters
>>>>>>> around in
>>>>>>> the BarrierSetC1 backend, instead of passing around the baked
>>>>>>> LIRAccess
>>>>>>> object with dummy parameters.
>>>>>> We also need the LIRGenerator* and CodeEmitInfo* (see below).
>>>>>> Would you
>>>>>> prefer to pass 4 arguments around? I am ok with either approach, and
>>>>>> passing for args is probably easier to understand than passing one
>>>>>> opaque LIRAccess&. Whatever you prefer.
>>>>>>
>>>>>>> 3) What do you need the CodeEmitInfo parameter for? It doesn't
>>>>>>> seem to
>>>>>>> be used anywhere. Or is it?
>>>>>> I believe we use it in case the obj is not known to be not NULL, in
>>>>>> which case we need to emit null-checks.
>>>>>>
>>>>>> I'll post an updated webrev once we agree on #2.
>>>>> ok, let's have a look at it:
>>>>>
>>>>> differential:
>>>>> http://cr.openjdk.java.net/~rkennke/JDK-8209667/webrev.01.diff/
>>>>> full:
>>>>> http://cr.openjdk.java.net/~rkennke/JDK-8209667/webrev.01/
>>>>>
>>>>> if you think that's better than to use the opaque access stuff, I'm
>>>>> fine
>>>>> with it. let me know how you like it.
>>>> So, Erik, what do you prefer, passing everything via LIRAccess or via
>>>> explicit parameters. I am leaning to explicit parameters, it just makes
>>>> it more obvious what it is, and requires less poking around to figure
>>>> out what is what, etc.
>>>>
>>>> Ok?
>>>>
>>>> Roman
>>>>
>>>>
>