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

[Jasmine Karthikeyan]

On Wed, 4 Jun 2025 05:43:28 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> 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.
>
>> Earlier in the function, this logic is guarded with vtn->basic_type() == T_INT so I think only integer nodes need to be added to the list. Let me know what you think!
> 
> That sounds reasonable. And that would mean we only have to add `int` operation to that assert. And if anybody ever relaxes that `vtn->basic_type() == T_INT` check, then they would immediately run into that assert. Would be nice.

I think this is an interesting point. My concern is that this might cause unexpected assert failures to occur in rare cases, which could create a large bug tail. The behavior in the non-truncation case should not cause miscompiles so I'm not sure that a general assert is warranted. The one outlying case is that unexpected `Type` nodes can cause the later `vt = container_type(in);` line to produce an invalid result since it uses the bottom type, so I think an assert for that specifically could be useful.

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

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