RFR: JDK-8209667: Explicit barriers for C1/LIR
Erik Österlund
erik.osterlund at oracle.com
Wed Aug 22 11:21:44 UTC 2018
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
>
>
More information about the hotspot-gc-dev
mailing list