RFR: JDK-8214055: GC/C2 abstraction for phaseX

[Roman Kennke]

> Why you need 2 separate functions which do almost the same?

The same can be asked about the enclosing methods there
(PhaseIterGVN::add_users_to_worklish() and PhaseCPP::analyze()). I guess
the outer semantics should equal the GC interface semantics.

> And why
> iterate over all nodes again?
> Can you have just one method which you call inside both loops?

You mean for ZGC. We probably could. But for Shenandoah we have a
completely different block of things to accomplish, so I went  for the
more general interface. Yeah, it means to 'duplicate' the loop, but it
seemed the lesser evil to me.

Thanks,
Roman

> void add_users_to_worklist(Unique_Node_List& worklist, Node* u) {
>   // Search for load barriers behind the load
>   for (DUIterator_Fast i3max, i3 = u->fast_outs(i3max); i3 < i3max; i3++) {
>     Node* b = u->fast_out(i3);
>     if (is_gc_barrier_node(b)) {
>       worklist.push(b);
>     }
>   }
> }

> 
> Thanks,
> Vladimir
> 
> On 11/21/18 2:53 AM, Roman Kennke wrote:
>> There are (Z)GC specific changes in
>> PhaseIterGVN::add_users_to_worklist() and PhaseCCP::analyze(), and
>> Shenandoah needs to add some stuff there too. Let's put in an
>> abstraction for this.
>>
>> It should be noted that the ZGC path is now unwoven from the general
>> path, at the expense of iterating the outputs 2x. I don't expect this to
>> be a problem.
>>
>> The way it was before I did not like much, because it built some fairly
>> ZGC specific implicit knowledge (has_load_barriers()) into the shared
>> path, the way it's done now it seems more explicitely ZGC path. It could
>> be argued that a future GC with load-barriers would need this too, but
>> we don't know this yet. It could just the same be the case that a future
>> GC has load-barriers, but needs slightly different handling there. I'd
>> even say that this type of implicit knowledge is worse than simply
>> guarding a path with #if INCLUDE_ZGC or similar, because the latter
>> actually screams "THIS IS GC SPECIFIC" while the current implicit way
>> doesn't.
>>
>> Also note that this includes a subtle bugfix: in the PhaseCPP::analyze()
>> path, the ZGC version pushed to _worklist, but every other code there
>> uses the local worklist instead.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8214055
>> Webrev:
>> http://cr.openjdk.java.net/~rkennke/JDK-8214055/webrev.00/
>>
>> Testing: tier1 ok
>>
>> Can I please get reviews?
>>
>> Thanks, Roman
>>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20181121/9f7d32b3/signature.asc>