On Tue, 30 Jun 2026 12:15:26 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_transform` continues execution and not create
> a new `InlineType` node. To do that, I propose recording that `vt` is
> created for `phi` rather than changing the graph.
>
> ---------
> - [x]...
test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestInlineTypeInfinitePushThruPhi.java
line 31:
> 29: * @modules java.base/jdk.internal.value
> 30: * java.base/jdk.internal.vm.annotation
> 31: * @run main/othervm -XX:+StressIGVN
> -XX:CompileCommand=compileonly,${test.main.class}::test -Xbatch
I think this requires `-XX:+UnlockDiagnosticVMOptions`, right?
test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestInlineTypeInfinitePushThruPhi.java
line 34:
> 32: * -XX:-UseOnStackReplacement -XX:StressSeed=3696073068
> -XX:CompileTaskTimeout=8000 ${test.main.class}
> 33: * @run main/othervm -XX:+StressIGVN
> -XX:CompileCommand=compileonly,${test.main.class}::test -Xbatch
> 34: * -XX:-UseOnStackReplacement
> -XX:CompileTaskTimeout=8000 ${test.main.class}
And `CompileTaskTimeout` is debug only.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2600#discussion_r3499489693
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2600#discussion_r3499497496
