RFR: 8309266: C2: assert(final_con == (jlong)final_int) failed: final value should be integer

[Emanuel Peter]

On Wed, 7 Jun 2023 12:33:48 GMT, Daohan Qu <duke at openjdk.org> wrote:

> This patch should fix [JDK-8309266](https://bugs.openjdk.org/browse/JDK-8309266). A `jtreg` test is also added. I'd appreciate any comments and reviews. Thanks in advance!
> 
> ## Problem Analysis
> 
> For the following program,
> 
> public class Test {
> 
>     static boolean flag;
> 
>     public static void main(String[] args) {
>         for (int i = 0; i < 10000; i++) {
>             flag = !flag;
>             test();
>         }
>     }
> 
>     public static void test() {
>         int limit = flag ? Integer.MAX_VALUE : 1000;
> 
>         int i = 0;
>         while (i < limit) {
>             i += 3;
>             if (flag) {
>                 return;
>             }
>         }
>     }
> }
> 
> A `LoopLimitNode` will be generated and its `Limit` input is a `PhiNode`, as depicted in the following picture.
> 
> <img width="384" alt="phi_as_limit" src="https://github.com/openjdk/jdk/assets/18374295/c561bb09-9789-45c2-8fa8-5bc690ea8046">
> 
> During `PhaseCCP`, the `LoopLimitNode::Value()` tries to calculate the constant final value:
> https://github.com/openjdk/jdk/blob/16ebf47fe3b0fac7b67acfa589a26abf8843306b/src/hotspot/share/opto/loopnode.cpp#L2289-L2301
> 
> The problem is that the assertion in `line 2299` could fail during CCP though it must hold true at the end of CCP. Here is the reason: `PhaseCCP` initializes all nodes with the type `TOP` and iterates in an "arbitrary" order. The following order may happen:
> 
> 28 IfTrue => 34 Region => 36 Phi => 195 LoopLimit => ... => 29 IfFalse
> 
> 1. In `ProjNode::Value()` (`IfTrue` inherits it), the type of `IfTrue` is set to `Type::CONTROL`
> https://github.com/openjdk/jdk/blob/fa791119f0b73cd1e110d6a62d3bed58fee5740a/src/hotspot/share/opto/multnode.cpp#L168-L171
> 
> 2. In `PhiNode::Value()`, only `28 IfTrue`'s correspondence `33 ConI` gets merged (as `29 IfFalse` has not been dealt with yet), then it has a value of `int:max`.
> https://github.com/openjdk/jdk/blob/fa791119f0b73cd1e110d6a62d3bed58fee5740a/src/hotspot/share/opto/cfgnode.cpp#L1269-L1277
> 
> 3. In `LoopLimitNode::Value()`, it finds its `Limit` input `36 Phi` is constant, which triggers the assertion, and the assertion fails since the final value calculated from that constant limit (`int:max`) overflows.
> 
> ## Solution
> Move the overflow check to the end of CCP, where it must not fail.

@quadhier Thanks for looking into this! This but is currently not assigned to you. Please always make sure that you have it assigned to you, or at least mention in JIRA that you are working on it. Currently, @enothum had it assigned and was also working on it.

Why is the overflow acceptable? Does that not mean that the calculation did something wrong?

In the example, we have

init_con = 3
limit_con = 2147483647 = max_jint
stride_con = 3
stride_m = stride_con - 1 = 2
trip_count = (limit_con - init_con + stride_m)/stride_con = 715827882
final_con = init_con + stride_con*trip_count = 2147483649 = max_jint + 2 (overflow!)
final_int = -2147483647 (overflow!)


Does that not mean that we mis-calculated the `trip_count`? If it was 1 less, we would not have an overflow. Would that not fix the issue in a simpler way?

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

PR Comment: https://git.openjdk.org/jdk/pull/14353#issuecomment-1580950081