RFR: 8251535: Partial peeling at unsigned test adds incorrect loop exit check

Tobias Hartmann thartmann at openjdk.java.net
Thu Oct 15 13:16:27 UTC 2020


C2's `PhaseIdealLoop::partial_peel` searches for loop exit tests on the induction variable as cut point for partial
peeling. If no suitable signed test is found and `PartialPeelAtUnsignedTests` is enabled (default), unsigned `i <u
limit` checks are used. Since the exit condition `!(i <u limit)` can be split into `i < 0 || i >= limit`,
`PhaseIdealLoop::insert_cmpi_loop_exit` either clones the lower or upper bound check and inserts it as cut point before
the unsigned test. For example:

  loop:
    i += 1000;
    if (i <u 10_000) {
        goto loop;
    }
    goto exit;
  exit:
    return i;

Is converted to:

  loop:
    i += 1000;
    if (!(i < 10_000)) {   <-- Loop exit test as cut point for partial peeling
        goto exit;
    }
    if (i <u 10_000) {
        goto loop;
    }
    goto exit;
  exit:
    return i;
    
Now the problem is that if the unsigned check is inverted, i.e. we exit if the check **passes**, the newly inserted
test is incorrect:
    
  loop:
    i += 1000;
    if (i <u 10_000) {
        goto exit;
    }
    goto loop;
  exit:
    return i;

Is converted to:

  loop:
    i += 1000;
    if (i < 10_000) {   <-- This exit condition is wrong! For example, we should not exit for i = -1.
        goto exit;
    }
    if (i <u 10_000) {
        goto exit;
    }
    goto loop;
  exit:
    return i;

This leads to incorrect results because the loop is left too early.

The fix is to simply bail out when the loop exit condition is `i <u limit` => `i >= 0 && i < limit` because it can't be
split into a single signed exit check.

Thanks,
Tobias

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

Commit messages:
 - Removed trailing whitespace
 - 8251535: Partial peeling at unsigned test adds incorrect loop exit check

Changes: https://git.openjdk.java.net/jdk/pull/681/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=681&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8251535
  Stats: 192 lines in 2 files changed: 191 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/681.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/681/head:pull/681

PR: https://git.openjdk.java.net/jdk/pull/681


More information about the hotspot-compiler-dev mailing list