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