RFR: 8360192: C2: Make the type of count leading/trailing zero nodes more precise [v3]
Emanuel Peter
epeter at openjdk.org
Tue Aug 19 14:05:46 UTC 2025
On Tue, 24 Jun 2025 07:34:32 GMT, Qizheng Xing <qxing at openjdk.org> wrote:
>> This is because our implementation does not accept 0 as an input. I suggest doing this at `count_leading_zeros`, it makes more sense and also aligns our behaviour with the well-known [`countr_zero`](https://en.cppreference.com/w/cpp/numeric/countr_zero.html) and [`countl_zero`](https://en.cppreference.com/w/cpp/numeric/countl_zero.html)
>
>> 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?
>
> @eme64 The explanation of @merykitty is right, the implementation of `count_leading_zeros` and `count_trailing_zeros` reject zero as the input.
>
> Perhaps we could open another PR to add zero support for these functions, since it's less relevant to this node type change and might require other changes to the code that calls them.
In `src/hotspot/share/utilities/count_leading_zeros.hpp`, it says that 0 behavior is undefined. Ok... but why do we do that? Is that a performance optimization ? If yes, is it really worth it? If there is no good reason not to handle 0, we should just handle it.
We have some tests in `test/hotspot/gtest/utilities/test_count_leading_zeros.cpp`.
It would be interesting to quickly check if any use of these methods could ever encounter zero, and then hit the assert. I would not be surprised if we found a bug here.
I think this would be a worth while cleanup task. I would prefer if we clean things up now, and don't just let more special handling code get integrated.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25928#discussion_r2285322703
More information about the hotspot-compiler-dev
mailing list