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