RFR: 8305324: C2: Wrong execution of vectorizing Interger.reverseBytes
Daohan Qu
duke at openjdk.org
Tue Apr 11 09:26:34 UTC 2023
On Tue, 11 Apr 2023 02:24:42 GMT, Pengfei Li <pli at openjdk.org> wrote:
>> This patch should fix [JDK-8305324](https://bugs.openjdk.org/browse/JDK-8305324).
>>
>> `SuperWord::compute_vector_element_type()` implemented in `jdk/src/hotspot/share/opto/superword.cpp` propagates backward a narrower integer type when the upper bits of the value are not needed. However, `Integer.reverseBytes()` depends on higher-order bits of an integer and should be prevented from being narrowed and vectorized. Instead, it needs to be treated like `Math.abs()` (which is represented by `Op_AbsI` in the following code).
>>
>> https://github.com/openjdk/jdk/blob/0243da2e4adc1b7ab6fcd5b10778532101158dce/src/hotspot/share/opto/superword.cpp#L3935-L3945
>>
>> I have tested this patch for tier 1-3 on x86-64.
>
> src/hotspot/share/opto/superword.cpp line 3944:
>
>> 3942: const Type* vt = vtn;
>> 3943: int op = in->Opcode();
>> 3944: if (VectorNode::is_shift_opcode(op) || op == Op_AbsI || op == Op_ReverseBytesI) {
>
> (another suggestion) This list may be longer and longer as we vectorize more operations. Shall we move this check into a static function of `VectorNode`, like `VectorNode::requires_higher_order_bits(op)`, and put the comments inside?
Good suggestion, I think a function can make this condition self-documented. Thank you.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13406#discussion_r1162539678
More information about the hotspot-compiler-dev
mailing list