Integrated: 8309204: Obsolete DoReserveCopyInSuperWord

[Emanuel Peter]

On Tue, 3 Oct 2023 07:33:10 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

> I'm removing `CountedLoopReserveKit` as discussed in https://github.com/openjdk/jdk/pull/14168 ([JDK-8308917](https://bugs.openjdk.org/browse/JDK-8308917)).
> 
> I'm also obsolating `DoReserveCopyInSuperWord`, as it has now no use any more.
> 
> **Context**
> 
> During `SuperWord`, we first analyze the loop, do all profitability and correctness checks for vectorization, and only in `SuperWord::output` do we finally modify the C2 graph.
> 
> However, there could still be some cases where we fail during graph modification. At that point, we have already done some modifications, and now have the followoing options:
> 
> 1. Crash (e.g. `ShouldNotReachHere`, or simply SIGSEGV because of `nullptr` access etc.)
> 2. Undo modifications, revert to before SuperWord.
> 3. Bail out of compilation.
> 
> With [JDK-8136725](https://bugs.openjdk.org/browse/JDK-8136725) (https://github.com/openjdk/jdk/commit/115afda88e55b62da296080178a42946701408be) option 2 was chosen. We make a copy of the whole loop, and if we chose to abort vectorization we would swap in the unmodified copy. It was primarily used for cases where asserts fail (in debug), and in product instead revert to the unmodified copy. The disadvantage to this approach is that we are requiring the cloning/copying of all superword-ed loops - and none of them are really expected to fail and use the copy. There were some additional cases where we had to revert for post-loop-vectorization, but this was recently removed anyway ([JDK-8311691](https://bugs.openjdk.org/browse/JDK-8311691)).
> 
> Additionally, the product flag `DoReserveCopyInSuperWord` can disable the copy/restore mechanism - and one would hit a `ShouldNotReachHere` instead. It is unclear why there should be this flag. It was on by default, and until [JDK-8311691](https://bugs.openjdk.org/browse/JDK-8311691) it also lead to some bugs (eg. [JDK-8308949](https://bugs.openjdk.org/browse/JDK-8308949) where we would crash if the flag was disabled).
> 
> **Argument**
> 
> I suggest that the flag `DoReserveCopyInSuperWord` and `CountedLoopReserveKit` introduce complexity for what are essentially very rare edge cases (where we hit asserts, but want to continue somehow in product mode).
> 
> We should instead chose option 3, and bail out of the compilation (rejecting the inconsistent graph), and retry the compilation without `SuperWord`. This increases the work for the edge cases that fail for some reason, but avoids the cloning/copying in all normal cases.
> 
> **CSR**
> 
> Since `DoReserveCopyInSuperWord` is a pro...

This pull request has now been integrated.

Changeset: 1ed9c76e
Author:    Emanuel Peter <epeter at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/1ed9c76ec8a76592203ce35f240f8753ba49307c
Stats:     346 lines in 11 files changed: 34 ins; 280 del; 32 mod

8309204: Obsolete DoReserveCopyInSuperWord

Reviewed-by: kvn, thartmann

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

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