RFR: 8374043: C2: assert(_base >= VectorMask && _base <= VectorZ) failed: Not a Vector [v2]

[Xiaohong Gong]

On Tue, 13 Jan 2026 09:11:25 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Xiaohong Gong has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Check "top" and revert the assertion changes
>
> src/hotspot/share/opto/vectornode.cpp line 1923:
> 
>> 1921:     Node* mask = in1->in(1);
>> 1922:     const TypeVect* mask_vt = mask->bottom_type()->isa_vect();
>> 1923:     if (mask_vt == nullptr) {
> 
> It is better to filter the exact `Type::TOP` instance and assert that otherwise, this must be a `TypeVect`. Additionally, if the type of the input is `Type::TOP`, we can eagerly return `C->top()` to kill it.

Hi @merykitty , @iwanowww , I'v updated the change to check `TOP` input and converted the changes for assertion. Please help take another look. Thanks!

>Additionally, if the type of the input is Type::TOP, we can eagerly return C->top() to kill it.

This makes sense to me. However, I would prefer to use `return nullptr` in this optimization. My concerns are:

1. In `Ideal()` implementations of other nodes that handle `top` inputs, the common pattern is to detect `top` and simply return `nullptr`. This is reasonable because `Ideal()` is meant for optimization/transformation, and avoiding changes when a node has a top input is usually safer. In many cases, updating a node to top is done in `Value()`, which seems like a more appropriate place for that kind of change.

2. Conceptually, a node should only need to check whether its own inputs are `top`. In this case, we are required to look through and check `in(1)->in(1)`, which is less natural and makes the logic more fragile.

3. The `Ideal()` contract is to return either a new node or the node itself for the GVN phase. `C->top()` is effectively an existing “global” node, not a newly created one. Even though IGVN treats top as a special case, introducing this here feels risky and could increase the chance of HotSpot crashes. Using `return nullptr` avoids this extra risk.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29057#discussion_r2697855374