RFR: 8309685: Fix -Wconversion warnings in assembler and register code [v3]
Coleen Phillimore
coleenp at openjdk.org
Wed Jun 14 21:09:01 UTC 2023
On Wed, 14 Jun 2023 18:17:49 GMT, Dean Long <dlong at openjdk.org> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Happy to add riscv, thanks.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14396#discussion_r1230170540
More information about the hotspot-dev
mailing list