RFR: 8310921: Fix -Wconversion warnings from GenerateOopMap [v2]
Coleen Phillimore
coleenp at openjdk.org
Tue Jun 27 13:20:23 UTC 2023
On Tue, 27 Jun 2023 06:47:16 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>>
>> David Holmes comments
>
> src/hotspot/share/oops/generateOopMap.cpp line 663:
>
>> 661:
>> 662: int GenerateOopMap::next_bb_start_pc(BasicBlock *bb) {
>> 663: int bbNum = (int)(bb - _basic_blocks + 1);
>
> If this is an offset into a memory region then shouldn't it be `intptr_t` or `size_t`?
I made bbNum an intptr_t.
> src/hotspot/share/oops/generateOopMap.cpp line 2060:
>
>> 2058: tty->print_cr (" Total : %3.3f sec.", GenerateOopMap::_total_oopmap_time.seconds());
>> 2059: tty->print_cr (" (%3.0f bytecodes per sec) ",
>> 2060: (float)GenerateOopMap::_total_byte_count / GenerateOopMap::_total_oopmap_time.seconds());
>
> I don't understand this change. The denominator is a double due to `seconds()` so the numerator will be promoted to double as well, and the whole expression is a double.
I thought it needed to be float, but should be double. Thanks for noticing this.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14666#discussion_r1243717909
PR Review Comment: https://git.openjdk.org/jdk/pull/14666#discussion_r1243718374
More information about the hotspot-dev
mailing list