RFR: 8343700: ceil_log2 should not loop endlessly [v5]

Kim Barrett kbarrett at openjdk.org
Tue Nov 19 15:51:48 UTC 2024


On Tue, 19 Nov 2024 15:35:37 GMT, Sonia Zaldana Calles <szaldana at openjdk.org> wrote:

>> 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.
>
> I had tested your implementation but I added a few more test cases for peace of mind :-) 
> 
> I don't think we can do a death test because that would only get triggered for debug builds.  I can't seem to find any in the code base either. 
> 
> Also fixed the T(0) issue. Thanks!

Regarding death tests - look for tests using of `TEST_VM_ASSERT` and `TEST_VM_ASSERT_MSG`.
They are only available in debug builds, and uses are wrapped in `#ifdef ASSERT`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22074#discussion_r1848614308


More information about the hotspot-dev mailing list