RFR(L): 8185556: [MVT] C2 compiler support for non-flattened value type fields

[Tobias Hartmann]

Hi Roland,

On 28.09.2017 18:00, Roland Westrelin wrote:
>>> Also, usually after the graph is constructed with GraphKit, there is a
>>> pass of PhaseRemoveUseless to disconnect useless nodes. Your new use of
>>> GraphKit during igvn is not followed by PhaseRemoveUseless. I wonder if
>>> it's a problem and if some untransformed node could stay connected to
>>> graph.
>>
>> Why is that different to the version that does not use the GraphKit?
>>
>> All nodes that were added by the GraphKit should end up being processed by a subsequent PhaseIterGVN::optimize() because
>> they are guaranteed to be added to the worklist - which is verified by PhaseIterGVN::verify_PhaseIterGVN(). IGVN should
>> be "smart" enough to kill dead nodes, right?
> 
> Sure if the node is transform()'ed but not always with something like:
> 
> Node* n = new Node(1);
> n->set_req(0, some_other_node);
> 
> if (some_condition) {
>    // n unused
> } else {
>    n = gvn.transform(n);
> }

Yes but Node::set_req would register the node as modified and we would at least fail with "modified node was not 
processed by IGVN.transform_old()" if it was not transformed.

I think the main reason why we have PhaseRemoveUseless after GraphKit is because during parsing we cannot aggressively 
cut off dead parts of the graph but after parsing (with IGVN) we can (and will).

> But that must not be a common code pattern.

Right and the current use of GraphKit is very limited. If we end up using it more and above code shape shows up, it 
should be straight forward to fix. In my opinion, this is bad style anyway.

Thanks,
Tobias