On Tue, 30 Jun 2026 14:34:52 GMT, Roland Westrelin <[email protected]> wrote:
>> We have the following subgraph
>>
>>
>> 48 ConP === 0
>> 61253 Phi === 228 _ 48
>> 61252 InlineType === _ 61253 61254 61255 61256
>> 61248 Phi === 242 61252 61253
>>
>>
>> `Phi` 61248 is transformed and `PushInlineTypeDown::do_transform` runs.
>>
>> A new value type is created:
>>
>>
>> 61259 Phi === 242 1 1
>> 61258 InlineType === _ 61259 61260 61261 61262
>> ```
>>
>> First input of `Phi` 61248 is processed:
>>
>>
>> 61259 Phi === 242 61253 1
>> 61258 InlineType === _ 61259 61260 61261 61262
>>
>>
>> The second input of the `Phi` 61248, `Phi` 61253, is processed which
>> causes a recursive call to `PushInlineTypeDown::do_transform`. A value
>> type is created:
>>
>>
>> 61264 Phi === 228 _ 1
>> 61263 InlineType === _ 61264 61265 61266 61267
>>
>>
>> In the uses of `Phi` 61253, `Phi` 61253 is replaced by `InlineType`
>> 61263.
>>
>> One such as use is`Phi` 61259. As a consequence, the value type of the
>> caller of this `PushInlineTypeDown::do_transform` is modified:
>>
>>
>> 61259 Phi === 242 61263 1
>> 61258 InlineType === _ 61259 61260 61261 61262
>>
>>
>> `PushInlineTypeDown::do_transform` for `Phi` 61253 runs to completion:
>>
>>
>> 61264 Phi === 228 _ 48
>> 61263 InlineType === _ 61264 61265 61266 61267
>>
>>
>> `PushInlineTypeDown::do_transform` for `Phi` 61248 runs to completion:
>>
>>
>> 48 ConP === 0
>> 61264 Phi === 228 _ 48
>> 61263 InlineType === _ 61264 61265 61266 61267
>> 61259 Phi === 242 61263 61264
>> 61258 InlineType === _ 61259 61260 61261 61262
>>
>>
>> Which is a shape identical to what `PushInlineTypeDown::do_transform`
>> started from. Transformation then happens with `Phi` 61259 which
>> produces the same shape etc.
>>
>> Things go wrong, I think, because of this code in
>> `PushInlineTypeDown::do_transform`:
>>
>>
>> if (_can_reshape) {
>> // Replace phi right away to be able to use the inline
>> // type node when reaching the phi again through data loops.
>> PhaseIterGVN* igvn = _phase->is_IterGVN();
>> for (DUIterator_Fast imax, i = phi->fast_outs(imax); i < imax; i++) {
>> Node* u = phi->fast_out(i);
>> igvn->rehash_node_delayed(u);
>> imax -= u->replace_edge(phi, vt);
>> --i;
>> }
>> igvn->rehash_node_delayed(phi);
>> assert(phi->outcnt() == 0, "should be dead now");
>> }
>>
>>
>> because it changes an already processed input of a `Phi` and adds an
>> `InlineType` where there was none. From the comment, the goal is to
>> handle cases where we run into that same `Phi` when
>> `PushInlineTypeDown::do_tran...
>
> Roland Westrelin has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - review
> - review
Thanks for the test case @chhagedorn
-------------
PR Comment: https://git.openjdk.org/valhalla/pull/2600#issuecomment-4845047378
