RFR: 8366845: C2 SuperWord: wrong VectorCast after VectorReinterpret with swapped src/dst type

[Emanuel Peter]

On Thu, 4 Sep 2025 15:08:04 GMT, Galder Zamarreño <galder at openjdk.org> wrote:

>> I have seen 3 manifestations of this bug:
>> 
>> 1. assert
>> 
>> # Internal Error (.../src/hotspot/cpu/x86/x86.ad:7640), pid=84140, tid=28419
>> # assert(UseAVX > 2 && VM_Version::supports_avx512dq()) failed: require
>> 
>> 
>> 2. assert
>> 
>> # Internal Error (.../src/hotspot/share/opto/vectornode.cpp:1601), pid=4022154, tid=4022168
>> # Error: assert(bt == T_FLOAT) failed
>> 
>> 
>> 3. Wrong result
>> When the feature was available but we used the wrong CastVector
>> 
>> It seems that [JDK-8346236](https://bugs.openjdk.org/browse/JDK-8346236) introduced reinterpret nodes to SuperWord:
>> 
>> 
>>   } else if (VectorNode::is_reinterpret_opcode(opc)) {
>>     assert(first->req() == 2 && req() == 2, "only one input expected");
>>     const TypeVect* vt = TypeVect::make(bt, vlen);
>>     vn = new VectorReinterpretNode(in1, vt, in1->bottom_type()->is_vect());
>> 
>> 
>> Sadly, the `src` and `dst` type are swapped. For JDK25 [JDK-8346236](https://bugs.openjdk.org/browse/JDK-8346236) this had no bad effect yet, since we only cast between HF and short, which are both based on short.
>> 
>> But with [JDK-8329077](https://bugs.openjdk.org/browse/JDK-8329077) we can now do reinterpret between I/F and between D/L. Here swapping has an effect, especially if it is followed by a cast:
>> The cast deterines its input type from the output type of the input node. If that was a reinterpret node with the wrong output type, **we would get a cast with the wrong src type**. We might do a double -> int cast instead of a long -> int cast. That leads to all sorts of issues.
>> 
>> The fuzzer test was only just recently added with [JDK-8324751](https://bugs.openjdk.org/browse/JDK-8324751). It uses MemorySegment, where unaligned float/double access gets handled with long/int memory access and then reinterpret (eg `MoveI2F`). But I was able to find examples that just work with `Float.intBitsToFloat` etc.
>
> Great catch @eme64! Sorry for introducing this issue :$
> 
> I was wondering if we'd need more cases being tested? Reversed ones? E.g. `test1 ` goes from long -> double -> float -> int, do we need something that does int -> float -> double -> long? Does that make sense?

@galderz Thanks for having a look.

We could add more cases, but I'd also like to integrate rather quickly since this is failing 10x or more on our CI daily.
If it takes too long we would have to back out [JDK-8329077](https://bugs.openjdk.org/browse/JDK-8329077) instead.

So I'd suggest this:
We can file a follow-up RFE that covers more cases. Because we basically need to cover: all Reinterpret (I2F, F2I, L2D, D2L, HF2S, S2HF) with all compatible casts after it. That is a lot of cases. We can consider using a templated test for it, or just generate them ahead.

Generally, it is quite difficult to test the "moves" well because of the way that different NaN bits are handled. I'd like to develop generally more templated tests. But it is difficult to do arbitrary expressions, because if you have some float expression that can generate a NaN, and then you "move" it to int with `Float.floatToRawIntBits`, you can get different results if you are in the interpreter or in compiled code.

The I2F, F2I, L2D, D2L are "moves" are currently also tested with unaligned memory accesses via MemorySegment - that is how we found this bug in the first place.

For now, I think the fix is quite simple and clear, so I'd think it is ok to defer the tests a little.

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

PR Comment: https://git.openjdk.org/jdk/pull/27100#issuecomment-3254226006