RFR: 8288564: C2: LShiftLNode::Ideal produces wrong result after JDK-8278114
Christian Hagedorn
chagedorn at openjdk.org
Fri Jun 17 08:38:53 UTC 2022
On Thu, 16 Jun 2022 16:33:30 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:
>> [JDK-8278114](https://bugs.openjdk.org/browse/JDK-8278114) added the following transformation for integer and long left shifts:
>>
>> "(x + x) << c0" into "x << (c0 + 1)"
>>
>> However, in the long shift case, this transformation is not correct if `c0` is 63:
>>
>>
>> (x + x) << 63 = 2x << 63
>>
>> while
>>
>> (x + x) << 63 --transform--> x << 64 = x << 0 = x
>>
>> which is not the same. For example, if `x = 1`:
>>
>> 2x << 63 = 2 << 63 = 0 != 1
>>
>> This optimization does not account for the fact that `x << 64` is the same as `x << 0 = x`. According to the [Java spec, chapter 15.19](https://docs.oracle.com/javase/specs/jls/se18/html/jls-15.html#jls-15.19), we only consider the six lowest-order bits of the right-hand operand (i.e. `"right-hand operand" & 0b111111`). Therefore, `x << 64` is the same as `x << 0` (`64 = 0b10000000 & 0b0111111 = 0`).
>>
>> Integer shifts are not affected because we do not apply this transformation if `c0 >= 16`:
>>
>> https://urldefense.com/v3/__https://github.com/openjdk/jdk19/blob/729164f53499f146579a48ba1b466c687802f330/src/hotspot/share/opto/mulnode.cpp*L810-L817__;Iw!!ACWV5N9M2RV99hQ!KmKUUYOAlI0VvywPQaVXdxu-rJyyxKvCWxQZwZqa9VKtndvSljVPuQnnTIr-YXQ3X_A6DmYzgb9jnFR9GGMGVr08uQe7Ck-V_g$
>>
>> The fix I propose is to not apply this optimization for long left shifts if `c0 == 63`. I've added an additional sanity assertion for integer left shifts just in case this optimization is moved at some point and ending up outside the check for `con < 16`.
>>
>> Thanks,
>> Christian
>
> src/hotspot/share/opto/mulnode.cpp line 816:
>
>> 814: if (add1->in(1) == add1->in(2)) {
>> 815: // Convert "(x + x) << c0" into "x << (c0 + 1)"
>> 816: assert(con != BitsPerJavaInteger - 1, "sanity check, optimization cannot be applied for con == 31");
>
> The assert is useless because there is check at line 812 `(con < 16).
Yes, that's true. I just wanted to make sure that we do not forget about the fact that we cannot apply this optimization for `con == 31` if this optimization is moved at some point or we decide to remove the `con < 16` restriction. But I guess I can also just add a comment instead.
-------------
PR: https://git.openjdk.org/jdk19/pull/29
More information about the hotspot-compiler-dev
mailing list