RFR: 8311847: Fix -Wconversion for assembler.hpp emit_int8,16 callers
Andrew Haley
aph at openjdk.org
Tue Jul 11 13:55:14 UTC 2023
On Tue, 11 Jul 2023 12:39:09 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
>> src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 556:
>>
>>> 554: } else {
>>> 555: i->f(0b01, 25, 24);
>>> 556: i->f(checked_cast<unsigned>(offset() >> size), 21, 10);
>>
>> I remember there being issues with checked_cast and sign extension. When going from int64_t to unsigned and back, I think we need to do int64_t --> int32_t --> unsigned, and not int64_t --> uint64_t --> unsigned. Is that what checked_cast will do? To be safe, or at least make it easier to understand, shound't we use checked_cast only to change the size or sign, but not both? So going to int64_t to unsigned would require two checked_casts.
>
> The assignment does the sign conversion first. The mask removes the top half with sign extension (right_n_bits is a macro that somehow returns intptr_t). So the check_cast<> just converts unsigned 64 bit to unsigned 32, which shouldn't be necessary since we just chopped off the top bits.
>
>
> uint64_t uval = val;
> unsigned mask = checked_cast<unsigned>(right_n_bits(nbits));
> uval &= mask;
> f(checked_cast<unsigned>(uval), lsb + nbits - 1, lsb);
That's right.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14822#discussion_r1259769005
More information about the hotspot-dev
mailing list