RFR: 8360192: C2: Make the type of count leading/trailing zero nodes more precise [v14]

[Emanuel Peter]

On Wed, 10 Sep 2025 07:03:02 GMT, Qizheng Xing <qxing at openjdk.org> wrote:

>> The result of count leading/trailing zeros is always non-negative, and the maximum value is integer type's size in bits. In previous versions, when C2 can not know the operand value of a CLZ/CTZ node at compile time, it will generate a full-width integer type for its result. This can significantly affect the efficiency of code in some cases.
>> 
>> This patch makes the type of CLZ/CTZ nodes more precise, to make C2 generate better code. For example, the following implementation runs ~115% faster on x86-64 with this patch:
>> 
>> 
>> public static int numberOfNibbles(int i) {
>>   int mag = Integer.SIZE - Integer.numberOfLeadingZeros(i);
>>   return Math.max((mag + 3) / 4, 1);
>> }
>> 
>> 
>> Testing: tier1, IR test
>
> Qizheng Xing has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove redundant import

Thanks for the update. Though I think you modified the test example so far that it does not work any more, i.e. it would not constant fold if the output range was wrong. I've identified 2 sources that would prevent constant folding:
- It is unclear if `getResultChecksum` would get inlined. That way, the `result` loses the type information about the ranges.
- The comparisons themselves would not constant fold, because the values you compare with are not constants, but array element loads. You need to compare `result` with a compile time constant.

Maybe the idea is not 100% clear for you:
Imagine `result` should be in some range `2..10`. But with a bug, we now return `3..10`. This means the output of `numberOfLeadingZeros` is still variable, and it does not constant fold. But: if there is something below it, like a `result < 3` ... this would now constant fold to `false`, even though we could have had a `2` at runtime.
But for this to work, it all needs to be in the same compilation unit, and the value we compare to `result` must be a compile time constant.

Does that make sense?

test/hotspot/jtreg/compiler/c2/gvn/TestCountBitsRange.java line 516:

> 514:     }
> 515: 
> 516:     int getResultChecksum(int result, int[] LIMITS) {

I would put a `@ForceInlinie` before this. You are using it in many methods, and so it may not get inlined reliably. And if it does not get inlined, then the result verifcation would not constant-fold, and so it would be kind of useless. Because we rely on the fact that if the range is wrong, we could get bad constant folding ;)

test/hotspot/jtreg/compiler/c2/gvn/TestCountBitsRange.java line 521:

> 519:             if (result < LIMITS[i]) sum += 1 << i;
> 520:             if (result > LIMITS[i + 1]) sum += 1 << (i + 1);
> 521:         }

I doublt that this works, because the test would not constant fold if the range was too narrow.
I think you need to manually unroll the loop, and load the constants from `static final` values, or another method that allows it to be a compile time constant.

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25928#pullrequestreview-3216519184
PR Review Comment: https://git.openjdk.org/jdk/pull/25928#discussion_r2344141663
PR Review Comment: https://git.openjdk.org/jdk/pull/25928#discussion_r2344145678