RFR: 8308917: C2 SuperWord::output: assert before bailout with CountedLoopReserveKit [v2]

[Emanuel Peter]

> 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()`.
> 
> There was one occurance I could not assert for: `vmask = create_post_loop_vmask();`. Read more below, there is actually a but there.
> 
> **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?
> 
> ------
> 
> **Bug: bad combination of -XX:+PostLoopMultiversioning -XX:-DoReserveCopyInSuperWord**
> 
> I filed it here: [JDK-8308949](https://bugs.openjdk.org/browse/JDK-8308949)
> 
> `PostLoopMultiversioning` unrolls the post-loop with the use of a vmask. Read more about post-loop vectorization here https://github.com/openjdk/jdk/pull/6828. But in `create_post_loop_vmask` we have some conditions which have to hold, and if they fail we get a `nullptr`, and bail out of SuperWord, via `CountedLoopReserveKit`.
> 
> But if we turn off `DoReserveCopyInSuperWord`, this is not cought, and we hit an assert.
> 
> Generally, this looks a bit unclean, what we have now: we should do the checks of `create_post_loop_vmask` before `SuperWord::output`,...

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

  remove rce post loop vmask assert -> it is legit there

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/14168/files
  - new: https://git.openjdk.org/jdk/pull/14168/files/ef8b5845..83531a0e

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

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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