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