RFR: 8309685: Fix -Wconversion warnings in assembler and register code

Kim Barrett kbarrett at openjdk.org
Tue Jun 13 13:34:53 UTC 2023


On Tue, 13 Jun 2023 06:01:42 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> This change widens some int arguments, narrows some int returns and adds some casts and checked_casts where the compiler was doing the implicit conversion.  I maintained existing types - ie if the parameter was short, I used a cast to short.  Also int is used in places that might be better served as unsigned int but I didn't fix that because it would be too large and risky.  The registers encode an offset in an array, so it's safe to checked_cast<> to get their encoding. This fix is limited so that the types changed and casts added are intentional.
>> See CR for counts of -Wconversion warnings this resolves.
>> Tested with tier1-7, also tested with tier1 on all Oracle supported platforms.
>
> src/hotspot/share/asm/assembler.hpp line 303:
> 
>> 301:   // These take full width arguments and cast down without checking. Many callers have sign extension converting up to
>> 302:   // full int width arguments, so this casts this away.
>> 303:   void emit_int8(   uint32_t x1)                                    { code_section()->emit_int8((uint8_t)x1); }
> 
> It is unfortunate that emit_int8 has that name, rather than emit_uint8.
> But that's probably water under the bridge; certainly so for this change.
> 
> It is also unfortunate that integral promotion and -Wconversion fight each
> other pretty hard.  And implicit conversions make it easy for -Wconversion
> failure cases to slip in while we don't have it enabled as part of normal
> operations. Here's an alternative approach:
> 
> 
> template<typename T, ENABLE_IF(std::is_same<T, uint8_t>::value)>
> void emit_int8(T v) { ... }
> 
> template<typename T,
>          ENABLE_IF(std::is_integral<T>::value),
>          ENABLE_IF(!std::is_same<T, uint8_t>::value)>
> void emit_int8(T v) { emit_int8(checked_cast<uint8_t>(v)); }
> 
> 
> Maybe that's not so bad?

The downside of this is it hides potentially real type mismatches from -Wconversion, once (if) that becomes
part of normal operations.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14396#discussion_r1228140415


More information about the hotspot-dev mailing list