RFR: 8267904: C2 crash when compile negative Arrays.copyOf length after loop [v4]

[Roland Westrelin]

On Wed, 2 Jun 2021 14:12:39 GMT, Hui Shi <hshi at openjdk.org> wrote:

>> src/hotspot/share/opto/graphKit.cpp line 3987:
>> 
>>> 3985:       ccast = _gvn.transform(ccast);
>>> 3986:       record_for_igvn(ccast);
>>> 3987:       replace_in_map(length, ccast);
>> 
>> _gvn.set_type_bottom(ccast);
>> is not needed then. 
>> record_for_igvn(ccast);
>> is likely not needed either. No sure the reason there was no call to transform() in the first place.
>
> @rwestrel 
> for "record_for_igvn" is likely not needed either.  Please help correct my understanding.
> 
> In GraphKit::new_array, CastIINode's _range_check_dependency is false and _gvn is not igvn. 
> When "_gvn.transform(ccast)" calls CastIINode::Ideal, it either already in for_igvn (goes to "phase->record_for_igvn(this)") or in "post_loop_opts_igvn". 
> 
> No need call record_for_igvn again after " _gvn.transform(ccast)".

The usual pattern at parse time is to call gvn.transform() and that's it. Later on, during igvn, if an input of a node changes, it's enqueued to be processed for igvn so at parse time, usually, there's no need to call record_for_igvn().
In this case, I think there's a call to record_for_igvn() because there's no call to gvn.transform(). The node has to be transformed at some point. If it's not done during gvn, it has to be during igvn and the cal to record_for_igvn() guarantees that.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4238