Integrated: 8251925: C2: RenaissanceStressTest fails with assert(!had_error): bad dominance

[Christian Hagedorn]

On Fri, 30 Oct 2020 11:28:10 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> The dominance failures start to occur after the fix for [JDK-8249749](https://bugs.openjdk.java.net/browse/JDK-8249749) which enabled the method `SWPointer::scaled_iv_plus_offset` to call itself recursively and walk the graph to match more instead of stopping immediately (no recursion):
> https://github.com/openjdk/jdk/commit/092389e3c91822b1e3f56f203cb7b90e84673f8e#diff-8f29dd005a0f949d108687dabb7379c73dfd85cd782da453509dc9b6cb8c9f81L3789-R3812
> 
> We check in `SWPointer::offset_plus_k` if a node is invariant and if so then we choose it as invariant. However, we now have cases in the Renaissance benchmarks where we select an invariant that is pinned to a `CastIINode` between the main and pre loop. An example is shown in the attached image. 5913 SubI is found as an invariant with the improved recursive search enabled by JDK-8249749. The control of 5913 SubI (with `get_ctrl`) is 5298 CastII. The problem is now that we use the invariant 5913 SubI in the pre loop limit check of  5281 CountedLoopEnd (done in `SuperWord::align_initial_loop_index`) because we assume that since the invariant is not part of the main loop, it can float into the pre loop. But this is prevented by 5298 CastII. This results in the dominance assertion failure when checking if the earliest control of 5270 Bool in the pre loop (5297 IfTrue because of 5913 SubI that is used by 5270 Bool) dominates the LCA of 5270 Bool (the pre loop header node).
> 
> My suggestion is to improve the invariant check in `SWPointer::offset_plus_k` to also check if the found invariant is not dominated by the pre loop end node. Repeated testing of the RenaissanceStressTest has not resulted in any dominance failures anymore.
> ![dominance_failure](https://user-images.githubusercontent.com/17833009/97696669-3752d200-1aa6-11eb-9a42-2e36550e2b8b.png)

This pull request has now been integrated.

Changeset: e4a32bea
Author:    Christian Hagedorn <chagedorn at openjdk.org>
URL:       https://git.openjdk.java.net/jdk/commit/e4a32bea
Stats:     102 lines in 2 files changed: 51 ins; 7 del; 44 mod

8251925: C2: RenaissanceStressTest fails with assert(!had_error): bad dominance

Reviewed-by: kvn, thartmann

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

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