RFR: 8343700: ceil_log2 should not loop endlessly [v5]
Kim Barrett
kbarrett at openjdk.org
Tue Nov 19 08:34:46 UTC 2024
On Mon, 18 Nov 2024 18:58:45 GMT, Sonia Zaldana Calles <szaldana at openjdk.org> wrote:
>> src/hotspot/share/utilities/powerOfTwo.hpp line 134:
>>
>>> 132: break;
>>> 133: }
>>> 134: }
>>
>> But I think this could all be much "simpler"? I think this works:
>>
>> template<typename T, ENABLE_IF(std::is_integral<T>::value)>
>> int ceil_log2(T value) {
>> assert(value > T(0), "Invalid value");
>> return log2i_graceful(value - 1) + 1;
>> }
>
> Sure, I updated to your suggestion instead.
The use of `T(0)` in the assert isn't necessary; just use `0`. Sorry about that.
Note that I didn't actually test that suggestion, just pencil and paper hand checked it. It looks like the new tests
cover the needed values though. Except shouldn't there be a death test for 0? Just one, I don't think you need
to death-test all covered types; death tests are kind of expensive to execute, since they need their own VM.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22074#discussion_r1847892247
More information about the hotspot-dev
mailing list