RFR: 8313882: Fix -Wconversion warnings in runtime code [v2]

[Patricio Chilano Mateo]

On Tue, 8 Aug 2023 20:09:56 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Here are fixes to silence -Wconversion warnings in runtime code.  Use direct cast for 64 bit int to double, otherwise default to checked_cast<>.  I changed the declaration of _held_monitor_count and _jni_monitor count to intx to fix cascade of warnings coming from this.
>> 
>> src/hotspot/share/runtime/synchronizer.cpp:1802:41: warning: conversion from 'intx' {aka 'long int'} to 'int' may change value [-Wconversion]
>>  1802 |     _thread->dec_held_monitor_count(rec + 1);
>>       |                                     ~~~~^~~
>> 
>> Tested with tier1-4 on linux-x64-debug and windows-x64-debug and tier1 on Oracle supported platforms.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Make bytecode.hpp index() functions return u2 not long.

src/hotspot/share/runtime/os.cpp line 143:

> 141:     milliseconds_since_19700101 / milliseconds_per_microsecond;
> 142:   const int milliseconds_after_second =
> 143:     checked_cast<int>(milliseconds_since_19700101 % milliseconds_per_microsecond);

Not related to this change but seems milliseconds_per_microsecond should be named milliseconds_per_second.

src/hotspot/share/runtime/safepoint.cpp line 98:

> 96:                                              int initial_number_of_threads,
> 97:                                              int threads_waiting_to_block,
> 98:                                              int iterations) {

Maybe leave iterations as unsigned and change the caller instead?

src/hotspot/share/runtime/sharedRuntime.cpp line 2348:

> 2346:     double sum = 0;
> 2347:     double weighted_sum = 0;
> 2348:     for (int i = 0; i <= n; i++) { sum += (double)histo[i]; weighted_sum += (double)(i*histo[i]); }

So hist is a uint64_t*, why can't we make sum and weighted_sum just uint64_t?

src/hotspot/share/runtime/threadHeapSampler.cpp line 359:

> 357:          "double and uint64_t do not have the same size");
> 358:   x = *reinterpret_cast<const uint64_t*>(&d);
> 359:   const uint32_t x_high = (uint32_t)(x >> 32);

Can't we do a checked_cast here? The cast should be reversible with that shift value.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15177#discussion_r1287725795
PR Review Comment: https://git.openjdk.org/jdk/pull/15177#discussion_r1287728163
PR Review Comment: https://git.openjdk.org/jdk/pull/15177#discussion_r1287734252
PR Review Comment: https://git.openjdk.org/jdk/pull/15177#discussion_r1287733638