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

Coleen Phillimore coleenp at openjdk.org
Wed Jun 14 21:27:58 UTC 2023


On Wed, 14 Jun 2023 21:05:58 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> src/hotspot/share/utilities/globalDefinitions.hpp line 530:
>> 
>>> 528: #endif
>>> 529:   return static_cast<T2>(thing);
>>> 530: }
>> 
>> I don't understand this change.  Isn't it confusing to treat unsigned types as signed here?  Unsigned types don't sign extend.  If emit_intXX can take signed values then shouldn't that be reflected in the types of the arguements? 
>> Also, -max should be -max - 1 for 2's complement, right?
>
> The conversion from an unsigned type to a smaller unsigned type should have just worked if you convert back, like the old checked_cast code did, but callers of emit_intxxx were sign extending so the conversion back didn't work.
> 
> Calls like emit_int8() are passed arguments like 0xf0, but there were multiple (~940) that are passed with something that did some bitwise calculation that was promoted up to int.
> 
> Maybe the emit_intxx functions should take signed int, and then I could just check range in the checked_cast() without converting the argument to signed.  That might work and still provide the range checking that we want.
> 
> Yes, it should be max-1.  Thank you for looking at this.

Changing checked_cast<> to not convert to a signed type for T1 leads to very puzzling compiler errors.  For these emit_int8 etc functions, maybe a straight cast is the best thing, and leaving the definition checked_cast<> alone.  I'd be happy to do that as I had in my first patch.

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

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


More information about the hotspot-dev mailing list