RFR: 8299975: Limit underflow protection CMoveINode in PhaseIdealLoop::do_unroll must also protect type from underflow [v3]

Emanuel Peter epeter at openjdk.org
Fri Jan 20 10:59:52 UTC 2023


> This is a change similar to Rolands https://github.com/openjdk/jdk/pull/4388 [JDK-8267399](https://bugs.openjdk.org/browse/JDK-8267399): After the limit is modified, its type is not specific enough. Roland added a `CastII` in `PhaseIdealLoop::do_range_check` to keep the type.
> 
> I have to do a similar thing for `PhaseIdealLoop::do_unroll` - but I can directly constrain the type of the `CMoveI` node:
> We decrease the limit: `limit = limit - stride`. The subtraction makes an old limit type `int:<=3` underflow to `int`. But the `CMoveI` is already there to prevent the new limit from underflowing - it just does not manage that it's condition together with the two values it conditionally picks never underflow. And so the type information becomes too wide.
> 
> This is a problem if we have the following loop (simplified from regression test `test_simple`):
> 
>     static void test_simple(boolean flag) {
>         int x = flag ? Integer.MIN_VALUE : 3;
>         // x has type "int:min..3" == "<=3"
>         // Leads to Peel and 2x Unroll
>         for (int i = 0; i < x; i++) {
>             iArr[i * 2] = 666 + i;
>         }
>     }
> 
> First, we have a loop with Phi `0..2`.
> 
> for i in 0..2:  {body} i++
> 
> This is then peeled:
> 
> i = 0; {body}
> for i in 1..2:  {body} i++
> 
> and 2x Unrolled
> 
> i = 0; {body}
> for i in 1..2:
>   Phi 1..2
>   {body}
>   i++
>   Phi 2..2 -> collapses to constant
>   {body}
>   i++ -> collapses to constant int:3
>   exit condition: i < limit   -> int:3 < limit_t
> 
> 
> If the `CMoveI` cannot determine itself that the type is `int:<=3`, then the exit condition does not fold away, and we are left with a broken loop (`LoopNode` exists, but has no `Phi` it can reach from the `LoopEndNode`). This triggers a SIGSEGV if `-XX:+TraceLoopOpts` is enabled, or any other optimization that assumes the loop-phi to be present.

Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:

  Refactoring/renaming after discussion with Christian

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/12092/files
  - new: https://git.openjdk.org/jdk/pull/12092/files/4858629c..c08e529c

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=12092&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=12092&range=01-02

  Stats: 42 lines in 1 file changed: 17 ins; 3 del; 22 mod
  Patch: https://git.openjdk.org/jdk/pull/12092.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12092/head:pull/12092

PR: https://git.openjdk.org/jdk/pull/12092


More information about the hotspot-compiler-dev mailing list