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

[Erik Österlund]

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