RFR: 8323972: C2 compilation fails with assert(!x->as_Loop()->is_loop_nest_inner_loop()) failed: loop was transformed

[Emanuel Peter]

On Thu, 22 Feb 2024 14:36:52 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> Long counted loop are transformed into a loop nest of 2 "regular"
> loops and in a subsequent loop opts round, the inner loop is
> transformed into a counted loop. The limit for the inner loop is set,
> when the loop nest is created, so it's expected there's no need for a
> loop limit check when the counted loop is created. The assert fires
> because, when the counted loop is created, it is found that it needs a
> loop limit check. The reason for that is that the limit is
> transformed, between nest creation and counted loop creation, in a way
> that the range of values of the inner loop's limit becomes
> unknown. The limit when the nest is created is:
> 
> 
>  111  ConL  === 0  [[ 112 ]]  #long:-9223372034707292158
>  106  Phi  === 105 20 94  [[ 112 ]]  #long:9223372034707292160..9223372034707292164:www !orig=72 !jvms: TestInaccurateInnerLoopLimit::test @ bci:12 (line 40)
>  112  AddL  === _ 106 111  [[ 122 ]]  !orig=[110]
>  122  ConvL2I  === _ 112  [[ ]]  #int
> 
> 
> The type of 122 is `2..6` but it is then transformed to:
> 
> 
>  106  Phi  === 105 20 154  [[ 191 130 137 ]]  #long:9223372034707292160..9223372034707292164:www !orig=[72] !jvms: TestInaccurateInnerLoopLimit::test @ bci:12 (line 40)
>  191  ConvL2I  === _ 106  [[ 196 ]]  #int
>  195  ConI  === 0  [[ 196 ]]  #int:max-1
>  196  SubI  === _ 195 191  [[ 201 127 ]]  !orig=[123]
> 
> 
> That is the `(ConvL2I (AddL ...))` is transformed into a `(SubI
> (ConvL2I ))`. `ConvL2I` for an input that's out of the int range of
> values returns TypeInt::INT and the bounds of the limit are lost. I
> propose adding a `CastII` after the `ConvL2I` so the range of values
> of the limit doesn't get lost.

Looks reasonable, but these ad-hoc CastII also make me nervous.

What worries me with adding such "Ad-Hoc" CastII nodes is that elsewhere a very similar computation may not have the same tight type. And then you have a tight type somewhere, and a loose type elsewhere. This is how we get the data-flow collapsing and the cfg not folding.

@rwestrel please wait for our testing to complete, I just launched it.

test/hotspot/jtreg/compiler/longcountedloops/TestInaccurateInnerLoopLimit.java line 40:

> 38: 
> 39:     public static void test() {
> 40:         for (long i = 9223372034707292164L; i > 9223372034707292158L; i += -2L) { }

I'm always amazed at how such simple tests can fail. Is there any way we can improve the test coverage for Long loops?

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

Marked as reviewed by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17965#pullrequestreview-1931732253
PR Comment: https://git.openjdk.org/jdk/pull/17965#issuecomment-1992220126
PR Review Comment: https://git.openjdk.org/jdk/pull/17965#discussion_r1521846724