RFR: 8373502: C2 SuperWord: speculative check uses VPointer variable was pinned after speculative check, leading to bad graph [v3]

[Tobias Hartmann]

On Tue, 16 Dec 2025 13:23:40 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Thanks for @chhagedorn and @rwestrel for triaging / doing some first investigation.
>> 
>> This is a regression of [JDK-8324751](https://bugs.openjdk.org/browse/JDK-8324751) / https://github.com/openjdk/jdk/pull/24278.
>> 
>> This is almost the same as https://github.com/openjdk/jdk/pull/28449, so have a quick look at it.
>> It was also an issue with some nodes being pinned too low, and not available at the speculative check.
>> There, it was the `pre_init` values of the `iv`. Now it is the variables of the `VPointer`.
>> The fix is pretty similar as well.
>> 
>> ------------------------------------------
>> 
>> **Analysis**
>> 
>> The reproducer gets a `bad graph` assert because of this cycle:
>> <img width="885" height="788" alt="image" src="https://github.com/user-attachments/assets/f2992470-5452-4fb2-841e-353f753d9cbd" />
>> Note: `921 CountedLoop` is the pre-loop, the main-loop is further down from it.
>> And `607 ParsePredicate` is the `#Auto_Vectorization_Check`, and `1403` is the aliasing check inserted for the VPointer named below.
>> 
>> This is the relevant VPointer:
>> `VPointer[size: 4, object, base(920 CastPP) + con( 20) + iv_scale( 0) * iv + invar(0)]`
>> The base `920 CastPP` is the problematic variable.
>> 
>> In `VPointer::init_are_non_iv_summands_pre_loop_invariant`, we check that:
>> `_vloop.is_pre_loop_invariant(variable)`
>> And that holds for `920 CastPP`. So far so good.
>> 
>> This used to be enough when we only adjusted the pre-loop limit for alignment.
>> But now that we need the variables for the aliasing runtime check further up, this is not sufficient any more. Analogue to https://github.com/openjdk/jdk/pull/28449, we would now need:
>> `this->_vloop.is_available_for_speculative_check(variable)`
>> And that is false for `920 CastPP`, since it is pinned after the speculative check.
>> 
>> **Solution**
>> We should not insert the aliasing runtime check, and hence we probably cannot vectorize this case.
>> 
>> For now, this makes all tests pass. I think just like with https://github.com/openjdk/jdk/pull/28449 these cases are edge cases we don't have to worry too much about. But if they ever do become important, we could try to uncast the variables. But I don't know if that is without issues, we would certainly lose some info that we get from the casts.
>
> Emanuel Peter has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8373502-SW-VPointer-variables-at-speculative-check
>  - Update test/hotspot/jtreg/compiler/loopopts/superword/TestAliasingCheckVPointerVariablesNotAvailable.java
>  - fix up detail
>  - JDK-8373502

Marked as reviewed by thartmann (Reviewer).

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

PR Review: https://git.openjdk.org/jdk/pull/28783#pullrequestreview-3587343969