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