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

[Tobias Hartmann]

On Thu, 4 Sep 2025 14:42:46 GMT, Emanuel Peter <epeter 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.

Looks good to me otherwise. Nice test!

test/hotspot/jtreg/compiler/loopopts/superword/TestReinterpretAndCast.java line 170:

> 168:             int v0 = a[i];
> 169:             float v1 = Float.intBitsToFloat(v0);
> 170:             // Reinterpret: int -> float

Same here.

test/hotspot/jtreg/compiler/loopopts/superword/TestReinterpretAndCast.java line 212:

> 210:             float v2 = v1.floatValue();
> 211:             int v3 = Float.floatToRawIntBits(v2);
> 212:             // Reinterpret: float -> int

The indentation is off here. Please also fix the whitespace errors.

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

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27100#pullrequestreview-3188100502
PR Review Comment: https://git.openjdk.org/jdk/pull/27100#discussion_r2324177886
PR Review Comment: https://git.openjdk.org/jdk/pull/27100#discussion_r2324177346