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

[Emanuel Peter]

On Mon, 23 Jun 2025 07:56:48 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:
> 
>   Narrow type bound

src/hotspot/share/opto/countbitsnode.cpp line 46:

> 44: static int count_trailing_zeros_long(jlong l) {
> 45:   return l == 0 ? BitsPerLong : count_trailing_zeros(l);
> 46: }

Can you explain why you need this?
Why is `count_trailing_zeros` and `count_leading_zeros` not enough, when you cast at the use-site?

src/hotspot/share/opto/countbitsnode.cpp line 53:

> 51:   if (t == Type::TOP) return Type::TOP;
> 52:   const TypeInt* ti = t->isa_int();
> 53:   if (ti) {

Implicit null check not allowed by style guide :)

https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md

> Do not use ints or pointers as (implicit) booleans with &&, ||, if, while. Instead, compare explicitly, i.e. if (x != 0) or if (ptr != nullptr), etc.

test/hotspot/jtreg/compiler/c2/irTests/TestCountBitsRange.java line 2:

> 1: /*
> 2:  * Copyright (c) 2025 Alibaba Group Holding Limited. All Rights Reserved.

Can you please not use the `irTests` directory, and instead put it in a more thematically relevant one? Maybe `gvn`?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25928#discussion_r2161282004
PR Review Comment: https://git.openjdk.org/jdk/pull/25928#discussion_r2161283097
PR Review Comment: https://git.openjdk.org/jdk/pull/25928#discussion_r2161285920