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

[Emanuel Peter]

On Tue, 10 Jun 2025 05:00:36 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:

>> 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?
>
> In this case I believe abs/shifts were added because they needed to differentiate between signed and unsigned loads when dealing with truncation (abs was added in [JDK-8261022](https://bugs.openjdk.org/browse/JDK-8261022)) but the other branch that promotes to int is used by all the other nodes. This logic is pretty confusing because it's doing 2 separate things in the same code path. From my understanding, `Op_ReverseBytesI` requires the promotion to int behavior because the issue lies with truncation. Though, I think adding some more tests here would be a good idea so I'll look into that.

Thanks :)
If it is indeed separate logic, you could also consider a separate refactoring and adding those tests there. But I leave it up to you, maybe it is simple enough to do it all here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25440#discussion_r2136932600