RFR(XL): 8224675: Late GC barrier insertion for ZGC

[Nils Eliasson]

On 2019-06-03 10:50, Roland Westrelin wrote:
> Hi Nils,
>
>>> loopnode.cpp:
>>>
>>> Do we really need a new entry in the gc api for barrier_insertion()
>>> couldn't this go under optimize_loops()?
>> It could. There is only remove_range_check_casts in between right now. I
>> choose to have it as its own for separation, in a similar way to how
>> LoopOptsNone are used.
> I think your answer is for code in compile.cpp but my question was about
> PhaseIdealLoop::build_and_optimize(). Do we need any change at all in
> this method or could existing gc api points be used instead?

Ah - I think I get it. You mean like this:

void ZBarrierSetC2::barrier_insertion_phase(PhaseIterGVN& igvn)const {
   PhaseIdealLoop ideal_loop(igvn,LoopOptsNone);

   // First make sure all loads between call and catch are moved to the 
catch block clean_catch_blocks(&ideal_loop);

   // Then expand barriers on all loads insert_load_barriers(&ideal_loop);

   // Handle all Unsafe that need barriers. insert_barriers_on_unsafe(&ideal_loop);

   // Cleanup any modified bits igvn.optimize();

   igvn.C->clear_major_progress();
}

An excellent idea. Then I can remove the new LoopOptsMode::BarrierInsertion.


>
>>> In fixup_uses_in_catch() wouldn't you need to deal with phis the way you
>>> do in call_catch_cleanup_one():
>>>
>>> 1019     // We found no single catchproj that dominated the use - The use is at a point after
>>> 1020     // where control flow from multiple catch projs have merged. We will have to create
>>> 1021     // phi nodes before the use and tie the output from the cloned loads together. It
>>> 1022     // can be a single phi or a number of chained phis, depending on control flow
>> No - it's a two step process.
>>
>> There is a load hanging off a call with multiple catch projs. (There can
>> be no other control-flow in between.) The load have uses, either (1) in
>> the catch blocks, or (2) in some cases in the call block, hanging off
>> the load.
>>
>> fixup_uses_in_catch responsibility is to turn (2) into (1). It clones
>> all uses of the load, in the call block, out to the catch blocks. This
>> is done recursively. There can never be any phis here.
> Ok.
>
>>> Is there a chance that a load would be processed by
>>> fixup_uses_in_catch()? I see call_catch_cleanup_one() is where they are
>>> expected to be handled but you only go there if
>>> load->is_barrier_required() is true. So could you have a load for which
>>> is_barrier_required() is true have a use for which is_barrier_required()
>>> is not true and both of them be in the catch block?
>> In theory yes. If the load without a barrier is a use of the load with a
>> barrier, it would be processed first. When the load with a barrier is
>> processed, it would clone the load without a barrier, like any other node.
> If I understand the logic right, in the case, that load would be cloned
> as many times as there are uses. But couldn't there be multiple uses in
> a control flow path causing a load to be executed multiple times which
> would be in violation of the java memory model?

I've been running some experiments with asserts on the clone code.

1) There can never be any control flow here - so now phis or such.

2) Stores have explicit control - and would never be scheduled here either.

3) Loads - they end up here because they can float. They only matter if 
there is a use dominated by the catch (after a merge of catch control 
flow), or uses in more than one catch-proj branch. The only nodes 
observed being cloned is LoadPNodes with barriers, BoolNodes, and CmpP 
nodes. It's the same pattern of comparing a pointer. All other load has 
it's control in the catch-projs.

I will add asserts to the clone in fixup_uses_in_catch to reflect this 
conclusion and make sure that I catch any change in behavior.

/ Nils

>
> Roland.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20190604/d8435232/attachment.html>