RFR: 8374889: C2 VectorAPI: must handle impossible combination of signed cast from float [v3]

[Quan Anh Mai]

On Wed, 14 Jan 2026 07:38:34 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> I found this bug with the Template Framework (Vector API Library extension): https://github.com/openjdk/jdk/pull/28873
>> 
>> In `VectorCastNode::opcode`, we have an assert that we **cannot** have an `unsigned cast from float`, it would be nonsense.
>> https://github.com/openjdk/jdk/blob/2fbe47559e9ba45306bd08c3636647f865a75abd/src/hotspot/share/opto/vectornode.cpp#L1490-L1491
>> 
>> When we intrinsify `VectorSupport.convert`, we get a constant from the VectorAPI, that determines if we have a signed or unsigned cast, and other constants that determine the `from` and `to` types.
>> 
>> At runtime, the VectorAPI implementation can (I assume) never take a path of `unsigned cast from float`.
>> But it seems that nothing prevents the VM from compiling such an (unreachable) path.
>> 
>> Here is how I think it happens:
>> - `AbstractVector::castShape` creates a `C` conversion (lane-wise conversion). So at runtime, we will take the `C` switch-case in `AbstractVector::convert0`.
>> - Profiling can also make the `Z` (lane-wise reinterpret) switch-case live, because of other cast and reinterpret calls that use that code path. And we may not (yet) have proven that the `Z` switch-case is never taken.
>> - So we end up compiling the `Z` path, and call `VectorSupport.convert` with types `float` and `long`. And since the `Z` path sees that the from size is smaller than the to size, we get a `UCAST` (zero extension). Hence, we try to intrinsify a vector unsigned cast from float, and hit the assert.
>> 
>> https://github.com/openjdk/jdk/blob/2fbe47559e9ba45306bd08c3636647f865a75abd/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/AbstractVector.java#L742-L743
>> 
>> That the `Z` path is unreachable for `castShape` seems to be an invariant that only the VectorAPI knows about, and the VM cannot directly know that and determine that it is dead code. Thus, **I propose that we just check for the condition when trying to intrinsify**, and refuse intrinsification if it is violated.
>> 
>> **Update: ** instead of not intrinsifying, we chose a stronger path: we intrinsify it with a `HaltNode` that should never be encountered at runtime. The reproducer I have eventually even is able to fold away the `HaltNode`, so it turns out it is indeed a dead path.
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   halt refactor by demand of reviewers

Marked as reviewed by qamai (Committer).

src/hotspot/share/opto/graphKit.cpp line 2205:

> 2203:   halt(control(), frameptr(),
> 2204:        "uncommon trap returned which should never happen",
> 2205:        false /* don't emit code in product, it is just a waste of code space */);

We could be more explicit here, other `HaltNode` often gets folded, while a `Halt` after an uncommon trap is not, and it is a frequent occurrence.

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

PR Review: https://git.openjdk.org/jdk/pull/29169#pullrequestreview-3670491963
PR Review Comment: https://git.openjdk.org/jdk/pull/29169#discussion_r2698200941