RFR: 8359678: C2: assert(static_cast<T1>(result) == thing) caused by ReverseBytesNode::Value() [v2]

[Manuel Hässig]

On Thu, 26 Jun 2025 07:55:23 GMT, Hannes Greule <hgreule at openjdk.org> wrote:

>> Fixes an assertion when passing an int larger than short/char to the corresponding reverseBytes method in a constant-folding scenario. By just using static_cast, we can ignore the upper bytes and just swap the lower bytes.
>> 
>> Using jasm, I added a test case that covers such inputs. It felt easier to test this way than the other scenarios mentioned in the bug report.
>> 
>> I also removed the redundant checked_cast calls from the int/long case; we already have the correct type there.
>> 
>> Please review. Thanks.
>
> Hannes Greule has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove classfile version

@SirYwell, thank you for fixing this. It looks good overall, but it would be good to add the package. I think we do this for all new tests.
I kicked off some testing and will let you know about the results.

src/hotspot/share/opto/subnode.cpp line 2031:

> 2029:     case Op_ReverseBytesUS: return TypeInt::make(byteswap(static_cast<jchar>(con->is_int()->get_con())));
> 2030:     case Op_ReverseBytesI:  return TypeInt::make(byteswap(con->is_int()->get_con()));
> 2031:     case Op_ReverseBytesL:  return TypeLong::make(byteswap(con->is_long()->get_con()));

Why are you dropping the `checked_cast` here? Were they just an abundance of caution before?

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

Changes requested by mhaessig (Committer).

PR Review: https://git.openjdk.org/jdk/pull/25988#pullrequestreview-2986310035
PR Review Comment: https://git.openjdk.org/jdk/pull/25988#discussion_r2184863934