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

[Jasmine Karthikeyan]

On Tue, 10 Jun 2025 05:18:19 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> 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.
>
>> My concern is that this might cause unexpected assert failures to occur in rare cases, which could create a large bug tail.
> 
> If it is only an assert, and you return the "conservative" answer in product, then it only affects debug. This means we don't have to be too worried about it when the assert fails. And it gives us a chance to look at that new node, and decide if truncation is ok or not. A good comment at the site of the assert would make fixing those assert bugs quite easy. It would be an assert that ensures we get better performance because we vectorize. I think this is easier than having to write IR tests for all possible nodes with all possible truncations, that would be the alternative I suppose. But with IR tests we might always miss an operation.

That makes sense. I didn't consider that we can use it to find optimizations/regressions easier, I think it's worthwhile to have an assert for it. I've pushed a commit that adds it.

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

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