RFR: 8299975: Limit underflow protection CMoveINode in PhaseIdealLoop::do_unroll must also protect type from underflow [v5]
Emanuel Peter
epeter at openjdk.org
Fri Jan 20 12:35:42 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:
added stress and random flags/tags
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/12092/files
- new: https://git.openjdk.org/jdk/pull/12092/files/b59ca892..e81d2569
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12092&range=04
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12092&range=03-04
Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 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