RFR: 8349139: C2: Div looses dependency on condition that guarantees divisor not null in counted loop

Roland Westrelin roland at openjdk.org
Thu Feb 13 16:35:26 UTC 2025


The test crashes because of a division by zero. The `Div` node for
that one is initially part of a counted loop. The control input of the
node is cleared because the divisor is non zero. This is because the
divisor depends on the loop phi and the type of the loop phi is
narrowed down when the counted loop is created. pre/main/post loops
are created, unrolling happens, the main loop looses its backedge. The
`Div` node can then float above the zero trip guard for the main
loop. When the zero trip guard is not taken, there's no guarantee the
divisor is non zero so the `Div` node should be pinned below it.

I propose we revert the change I made with 8334724 which removed
`PhaseIdealLoop::cast_incr_before_loop()`. The `CastII` that this
method inserted was there to handle exactly this problem. It was added
initially for a similar issue but with array loads. That problem with
loads is handled some other way now and that's why I thought it was
safe to proceed with the removal.

The code in this patch is somewhat different from the one we had
before for a couple reasons:

1- assert predicate code evolved and so previous logic can't be
  resurrected as it was.
  
2- the previous logic has a bug.

Regarding 1-: during pre/main/post loop creation, we used to add the
`CastII` and then to add assertion predicates (so assertion predicates
depended on the `CastII`). Then when unrolling, when assertion
predicates are updated, we would skip over the `CastII`. What I
propose here is to add the `CastII` after assertion predicates are
added. As a result, they don't depend on the `CastII` and there's no
need for any extra logic when unrolling happens. This, however,
doesn't work when the assertion predicates are added by RCE. In that
case, I had to add logic to skip over the `CastII` (similar to what
existed before I removed it).

Regarding 2-: previous implementation for
`PhaseIdealLoop::cast_incr_before_loop()` would add the `CastII` at
the first loop `Phi` it encounters that's a use of the loop increment:
it's usually the iv but not always. I tweaked the test case to show,
this bug can actually cause a crash and changed the logic for
`PhaseIdealLoop::cast_incr_before_loop()` accordingly.

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

Commit messages:
 - fix & test

Changes: https://git.openjdk.org/jdk/pull/23617/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=23617&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8349139
  Stats: 137 lines in 6 files changed: 104 ins; 27 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/23617.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/23617/head:pull/23617

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


More information about the hotspot-compiler-dev mailing list