RFR: 8208601: Introduce native oop barriers in C2 for OopHandle

[Erik Osterlund]

Hi Vladimir,

Thank you for reviewing.

Incremental:
http://cr.openjdk.java.net/~eosterlund/8208601/webrev.00_01/

Full:
http://cr.openjdk.java.net/~eosterlund/8208601/webrev.01/

> On 01 Aug 2018, at 20:03, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
> Hi, Erik
> 
> 1. is_gc_barrier_node(b) is virtual so even if it return 'false' I am not sure C++ will collapse new search loops in phaseX.cpp and checks in other cases. Can we use some static check to allow C++ collapse that code in case there is no load barriers?

I see your point. I introduced a has_load_barrier() function in BarrierSetC2 that I can check outside of the loop, to make the extra load barrier search conditional. Was it something like this you had in mind?

> 2. Can you explain Phi case more. I would like to see an assert which verify cush shape of code to avoid cases when Phi come from loop, for example.

Sure. The load barrier has different shapes before and after macro expansion. Before macro expansion it is just a LoadBarrierNode. After expanding the barrier, the pseudo code for the inflated barrier looks something like this: value = is_bad(some_oop) ? slowpath_load : some_oop. The value in that pseudo code is the particular phi I am matching. What is unique about this phi (corresponding to the barrier region node depicting the good_or_null vs bad control flows), compared to an arbitrary phi, is that in(1) is the slowpath_load (LoadBarrierSlowPathRegNode) taken if the oop is bad. A phi in e.g. a loop would point to another phi with this property inside e.g. the loop body, never directly to a slowpath_load (LoadBarrierSlowPathRegNode).

Note that there is a basic and optimized macro expansion of the load barrier. The matching I use catches the optimized variant only (which uses the LoadBarrierSlowPathRegNode for the slow path). The basic barrier expansion is only used for atomic xchg (because self-healing the pointer would be an error in that path).

In the new version, I added some assertions right where load barriers are produced and expanded, to assert that it matches and can be stepped over reliably. That way, if we change the barriers, the matching code will not silently stop working. To make these asserts work, I added support for matching barriers on weak barriers as well, that use LoadBarrierWeakSlowPathRegNode. While that isn’t needed for my purposes for loading the mirror, it is better to be more complete. Now you can see at the assertions in ZBarrierSetC2::expand_loadbarrier_optimized the exact phi that I am matching.

I have still intentionally left matching of the “basic” load barrier used by atomic xchg for another day, as nobody has any need for that yet, and it is trickier to match.

BTW, it would be nice if in C2 the Node class had a Node* _expanded_from member that could track the node that macro expanded a node, and remember that through cloning of nodes, so that things like this would reduce to simply node->expanded_from()->is_LoadBarrier(), rather than trying to recognize the shape of the expanded barriers. Perhaps that would be useful for others? I did try something like that originally (but tracked with a hashtable), but unfortunately ran into trouble with cloned nodes not retaining the information, and made the decision to go for the shape recognition solution instead. But perhaps it could be generally useful for other shape recognition problems as well… But that might be outside the scope of this RFE.

Thanks,
/Erik
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180802/5c0a4a93/attachment.html>