RFR: 8308917: C2 SuperWord::output: assert before bailout with CountedLoopReserveKit

Emanuel Peter epeter at openjdk.org
Fri May 26 05:02:11 UTC 2023


In SuperWord::output we create a CountedLoopReserveKit, so that we can reverse edits to the loop, in case something goes wrong. As far as I understand all of these conditions should never occur, prior condition checking in SuperWord should have already verified that. We should at least add asserts so that we can catch such failures and fix them, and do not just silently bail out of SuperWord (reverse the graph to before SuperWord and continue compilation).

`DoReserveCopyInSuperWord` enables `do_reserve_copy()`. It is a product flag and default true. If it is disabled, and there is such a failure we just hit a `ShouldNotReachHere()`.

**Testing**

TODO testing up to tier6 plus stress testing.
(it already passed tier3 and stress testing)

**Discussion**

Do we really want to keep the `DoReserveCopyInSuperWord` flag (product, always true), which enables the use of `CountedLoopReserveKit`? It means that we always duplicate the loop (and the loops can be rather large because they were unrolled before SuperWord). It seems a bit of an edge case to want to bail out of SuperWord, but not of the whole compilation.
We can later decide if it makes sense to clone the whole loop via CountedLoopReserveKit (the loops can be large!), or if we should just have a regular compilation bailout instead (could simplify the code and reduce overhead of loop cloning).

Plus: it seems the checks and bailouts are very selectively applied. I don't see why we would nullptr check some "vector_opd" but not all of them. So if we decide to keep it, we should probably apply it more consistently.

What do you think?

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

Commit messages:
 - 8308917: C2 SuperWord::output: assert before bailout with CountedLoopReserveKit

Changes: https://git.openjdk.org/jdk/pull/14168/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14168&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8308917
  Stats: 5 lines in 1 file changed: 5 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/14168.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14168/head:pull/14168

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


More information about the hotspot-compiler-dev mailing list