RFR: 8305324: C2: Wrong execution of vectorizing Interger.reverseBytes [v4]

[Vladimir Kozlov]

On Tue, 11 Apr 2023 17:07:36 GMT, Daohan Qu <duke at openjdk.org> wrote:

>> src/hotspot/share/opto/vectornode.cpp line 440:
>> 
>>> 438: // of its higher order bits/bytes
>>> 439: bool VectorNode::requires_higher_order_bits_of_integer(int opc) {
>>> 440:   if (is_shift_opcode(opc) && opc != Op_LShiftI) {
>> 
>> Checking Op_LShiftI here will change behavior for `Short s = LShiftI(LoadB)` case and similar. May be it is okay and previous code worked because we did not vectorize due to different sizes of destination and Load.
>
> Thanks for your review. Yes, I see. AFAICS, the if-condition calling this function wants to check whether higher order bits are needed. So I distill the condition content into a function. The `Op_LShiftI` is excluded since it doesn't need such info. Do I miss something? Or should the if-condition have checked more than what I thought?

What I am trying to say is that before this change `vt` will be set to `velt_type(load)` even if `in` is `LShiftI` node. With your changes `vt` will stay `== vtn` if  `in` is `LShiftI` node.  `velt_type(load)` could be different from `vtn` and as result your change may introduce difference in code generation in other than `ReverseBytesI` cases.
This needs to be tested to see if number of generated vectors is not reduced for such cases.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13406#discussion_r1163152069