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

[Jasmine Karthikeyan]

On Wed, 18 Jun 2025 07:42:08 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Jasmine Karthikeyan has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>> 
>>  - Merge, address code review
>>  - Merge branch 'master' into vector-truncation
>>  - Add assert for unexpected node in truncation
>>  - Reformat, add comments and char tests
>>  - Fix vector truncation with subword types
>
> src/hotspot/share/opto/superword.cpp line 2549:
> 
>> 2547: 
>> 2548:   // For shorts and chars, check an additional set of nodes.
>> 2549:   if (type->isa_int() == TypeInt::SHORT || type->isa_int() == TypeInt::CHAR) {
> 
> What is this change for?

Hmm, this is an interesting case. After I merged from master, I saw an error in my clangd linter that `const Type*` can't be compared with `const TypeInt*`, since `TypeInt::SHORT/CHAR` were changed from `Type*` to `TypeInt*` by JDK-8315066. Though, when compiling it looks like it still compiles fine, which makes sense since the types are related. I think this might just be a mistake with my linter, so I've pushed a revert of the change.

> src/hotspot/share/opto/superword.cpp line 2594:
> 
>> 2592:     // and then add it to the list of truncating nodes or the list of non-truncating ones just above.
>> 2593:     // In product, we just return false, which is always correct.
>> 2594:     assert(false, "Unexpected node in SuperWord truncation: %s", NodeClassNames[in->Opcode()]);
> 
> Suggestion:
> 
>     // If this assert is hit, that means that we need to determine if the node can be safely truncated,
>     // and then add it to the list of truncating nodes or the list of non-truncating ones just above.
>     // In product, we just return false, which is always correct.
>     assert(false, "Unexpected node in SuperWord truncation: %s", NodeClassNames[in->Opcode()]);
> 
> Suggestion:
> 
>     // If this assert it hit, that means that we need to determine if the node can be safely truncated,
>     // and then add it to the list of truncating nodes or the list of non-truncating ones just above.
>     // In product, we just return false, which is always correct.
>     assert(false, "Unexpected node in SuperWord truncation: %s", NodeClassNames[in->Opcode()]);
> 
> Probably my bad previous suggestion is the culprit here 🙈

Ah whoops, I didn't catch that! I've pushed a typo fix.

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

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