RFR: 8350177: C2 SuperWord: Integer.numberOfLeadingZeros, numberOfTrailingZeros, reverse and bitCount have input types wrongly turncated for byte and short [v2]

[Emanuel Peter]

On Wed, 4 Jun 2025 04:28:38 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:

>> Hi all,
>> This patch fixes cases in SuperWord when compiling subword types where vectorized code would be given a narrower type than expected, leading to miscompilation due to truncation. This fix is a generalization of the same fix applied for `Integer.reverseBytes` in [JDK-8305324](https://bugs.openjdk.org/browse/JDK-8305324). The patch introduces a check for nodes that are known to tolerate truncation, so that any future cases of subword truncation will avoid creating miscompiled code.
>> 
>> The patch reuses the existing logic to set the type of the vectors to int, which currently disables vectorization for the affected patterns entirely. Once [JDK-8342095](https://bugs.openjdk.org/browse/JDK-8342095) is merged and automatic casting support is added the autovectorizer should automatically insert casts to and from int, maintaining correctness.
>> 
>> I've added an IR test that checks for correctly compiled outputs. Thoughts and reviews would be appreciated!
>
> Jasmine Karthikeyan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Reformat, add comments and char tests

@jaskarth Thanks for the updates! I'll run some testing now :)

src/hotspot/share/opto/superword.cpp line 2519:

> 2517: 
> 2518:   // Default to disallowing vector truncation
> 2519:   return false;

I was wondering:
We could have an assert here that lists all operations that cannot be truncated?
So if a new operation is added, then we will catch that it is not handled here yet, and we can add tests, and either allow it to truncate, or add it to the list of non-truncatable operations.

src/hotspot/share/opto/superword.cpp line 2579:

> 2577:               Node* load = in->in(1);
> 2578:               // For certain operations such as shifts and abs(), use the size of the load if it exists
> 2579:               if ((VectorNode::is_shift_opcode(op) || op == Op_AbsI) && load->is_Load() &&

Can you say a little more about this? What about `Op_ReverseBytesI`, did that not previously also get through here?

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

PR Review: https://git.openjdk.org/jdk/pull/25440#pullrequestreview-2895284484
PR Review Comment: https://git.openjdk.org/jdk/pull/25440#discussion_r2125689822
PR Review Comment: https://git.openjdk.org/jdk/pull/25440#discussion_r2125694632