RFR: 8311847: Fix -Wconversion for assembler.hpp emit_int8,16 callers

[Coleen Phillimore]

On Tue, 11 Jul 2023 02:28:07 GMT, Dean Long <dlong at openjdk.org> wrote:

>> Please review changes to fix -Wconversion warnings that come from assembler_<cpu>.cpp by adding narrow_casts to the emit_int8,16,24, and 32 functions.  And some other fixups with checked_cast.
>> 
>> Ran tier1 on Oracle platforms, and tier1-4 on linux-x64-debug, linux-aarch64-debug, windows-x64-debug.
>
> 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);

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14822#discussion_r1259674845