RFR: JDK-8210187: Explicit barriers for C2

[Roman Kennke]

>>>>
>>>> On 08/30/2018 05:58 PM, Roman Kennke wrote:
>>>>>>>  From a far away interface design perspective, those are still heap
>>>>>> accesses and GCs might be interested in intercepting e.g. final
>>>>>> values.
>>>>>> Maybe keep the resolves there and augment them with something like
>>>>>> ACCESS_FINAL and/or ACCESS_STABLE and let the backend decide what
>>>>>> to do
>>>>>> with it? For example, in Shenandoah we have an option to optimize for
>>>>>> the final/stable stuff and exploit the UD in the JMM in our
>>>>>> favour, or
>>>>>> to be conservative in favour of crazy applications, that could
>>>>>> react on
>>>>>> such a decorator. WDYT?
>>>>>>
>>>>> If final and stable properties can be exploited by the GC, then
>>>>> annotating that explicitly with decorators makes sense to me. However,
>>>>> it seems more intuitive to me to make such annotations in the relevant
>>>>> accesses, and not in a resolve barrier. Or what do you think?
>>>>
>>>> I think we need barriers by default, because we don't know what GC
>>>> would do. There is no knowledge
>>>> that it is safe to read the backing String arrays on this level. I
>>>> agree that additional decorator
>>>> tags about the nature of access may help GC to decide what to do
>>>> with it. That feels like a good
>>>> follow-up, which requires introducing resolve barriers anyway.
>>>
>>> I suppose that is true, despite it being a bit weird that we are
>>> cluttering the code with barrier hooks that 0 GCs (Shenandoah
>>> included) require. I suppose I am okay with that though.
>>>
>>>> I think current placement is beneficial, because otherwise we need
>>>> to put barriers into the each of
>>>> platform-specific assemblers that expand the relevant String
>>>> intrinsic nodes. Which appears more
>>>> tedious, and it makes barriers opaque for high-level optimizations,
>>>> e.g. folding the consecutive GC
>>>> barriers. (This seems to be the reason to have BarrierSetC2 to begin
>>>> with!)
>>>
>>> Yeah I don’t think we want to go down that route either.
>>>
>>> Looks good. Ship it!
>>
>> Thank you.
>>
>> In off-list discussions, Roland pointed out that some of those
>> resolve()s are only there to support object equality. Those should be
>> treated differently, with separate API(s) in BarrierSetC2. I excluded
>> those from the changeset:
>>
>> Incremental:
>> http://cr.openjdk.java.net/~rkennke/JDK-8210187/webrev.01.diff/
>> Full:
>> http://cr.openjdk.java.net/~rkennke/JDK-8210187/webrev.01/
> 
> Just a quick request. Please try to follow the coding style of the
> class/file when you're changing it. For example in barrierSetC2.hpp I
> see you added this, which now sticks out as it doesn't follow the style
> of the surrounding functions.
> 
> +  virtual Node* resolve(GraphKit* kit, Node* n, DecoratorSet
> decorators) const {
> +    return n;
> +  }
> +
> 
> So, please make it a one liner, like the other functions. I happen to
> personally prefer the multi-line style, but not when it goes against
> what every other function in the class looks like. Let's keep things
> consistent, at least within the same class/file. Thanks!
> 

Thanks Per for reviewing! I fixed that and will push the following
changeset after it came back from submit-repo clean:

http://cr.openjdk.java.net/~rkennke/JDK-8210187/webrev.02/

Thanks,
Roman

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20180831/0105c6ac/signature.asc>