RFR: JDK-8210656: Object equals abstraction for BarrierSetC2

[Erik Österlund]

Hi Roman,

I'm glad this idea works well for you. If you need an Ideal hook for 
CmpPNode anyway for barrier optimizations, then I suppose we should sort 
something out there. In that API though, it would be great if it was not 
specific to CmpPNode. I'm thinking something along the lines of Node* 
BarrierSetC2::ideal_node(Node* n), and then figure out if something 
should or should not be done for a given node in the backend. That way, 
if we need ideal hooks for other node types, we could reuse the same API 
to ask the BarrierSetC2 if it has more ideal nodes. What do you think?

Thanks,
/Erik

On 2018-09-13 14:15, Roman Kennke wrote:
> Hi Erik,
>
> I talked to Roland about this. It turns out that we already have this
> optimization pass, and could just as well live with cmp(resolve(a),
> resolve(b)). We need a little (shenandoah-specific-) hook in
> CmpPNode::Ideal() for that though (but we'd need that anyway I suppose).
> If you agree with that, I'll withdraw this RFR. Ok?
>
> Roman
>
>> Hi Roman,
>>
>> Interesting. So semantically, this is cmp(resolve(a), resolve(b)), but
>> you have circumstances in which the barriers are unnecessary and can be
>> elided. Any of them having null in their type is one reason, but I
>> suppose there are surely other reasons as well (such as finding
>> dominating write barriers).
>>
>> I see two different approaches for this barrier elision:
>> 1) Elide it during parsing (as you propose)
>> 2) Elide it during Optimize (which I think conceptually looks like a
>> natural fit)
>>
>> I originally proposed a function on BarrierSetC2 that I think I called
>> optimize_barriers() or something like that. The idea was to use this
>> hook to let GC barrier code shave off pointless (not to be confused with
>> useless) barriers that can be removed. Roland thought that this seemed
>> too specific to ZGC to warrant a general API, and I agreed, because
>> indeed only ZGC used this hook at the time. This is today
>> ZBarrierSetC2::find_dominating_barriers which is called straight from
>> Optimize.
>>
>> I wonder if it would make sense to re-instate that hook. Then you could
>> use the existing resolve() barriers during parsing, and leave barrier
>> elision tricks (null checks included, plus other tricks you might have
>> up your sleeve) to Optimize. For example, you might be able to walk your
>> list of barriers and disconnect these pointless barriers. What do you
>> think?
>>
>> Thanks,
>> /Erik
>>
>> On 2018-09-12 22:11, Roman Kennke wrote:
>>> This introduces an abstraction to deal with object equality in
>>> BarrierSetC2. This is needed by GCs that can have different copies of
>>> same objects alive like Shenandoah.
>>>
>>> The approach chosen here is slightly different than we did in e.g.
>>> BarrierSetAssembler and the runtime Access API: instead of owning the
>>> whole equality, it only provides a resolve-like method to resolve the
>>> operands to stable values. The reason for doing this is that it's easier
>>> to do this way in intrinsics if those barriers are detached from the
>>> actual CmpP. This is because the barriers create new memory states, and
>>> we'd have to create memphis around those things, which is considerably
>>> more complex.
>>>
>>> I chose to add a new resolve_for_obj_equals(a, b) method instead of
>>> using two calls to resolve(a); resolve(b); because this allows for
>>> optimization: if any of a or b is known to be NULL, we can elide
>>> barriers for both. This is not possible to do with two independent
>>> resolve() calls.
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8210656
>>> Webrev:
>>> http://cr.openjdk.java.net/~rkennke/JDK-8210656/webrev.00/
>>>
>>> Testing: passes hotspot/jtreg:tier1
>>>
>>> What do you think about this?
>>>
>>> Thanks,
>>> Roman
>>>
>