RFR: 8347555: [REDO] C2: implement optimization for series of Add of unique value [v5]

[Emanuel Peter]

On Fri, 28 Feb 2025 18:00:14 GMT, Kangcheng Xu <kxu at openjdk.org> wrote:

>> [JDK-8347555](https://bugs.openjdk.org/browse/JDK-8347555) is a redo of [JDK-8325495](https://bugs.openjdk.org/browse/JDK-8325495) was [first merged](https://git.openjdk.org/jdk/pull/20754) then backed out due to a regression. This patch redos the feature and fixes the bit shift overflow problem. For more information please refer to the previous PR.
>> 
>> When constanlizing multiplications (possibly in forms on `lshifts`), the multiplier is upgraded to long and then later narrowed to int if needed. However, when a `lshift` operand is exactly `32`, overflowing an int, using long has an unexpected result. (i.e., `(1 << 32) = 1` and `(int) (1L << 32) = 0`)
>> 
>> The following was implemented to address this issue.
>> 
>> if (UseNewCode2) {
>>     *multiplier = bt == T_INT
>>         ? (jlong) (1 << con->get_int()) // loss of precision is expected for int as it overflows
>>         : ((jlong) 1) << con->get_int();
>> } else {
>>     *multiplier = ((jlong) 1 << con->get_int());
>> }
>> 
>> 
>> Two new bitshift overflow tests were added.
>
> Kangcheng Xu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove tri-conditionals

This looks interesting! Thanks @tabjy for the work!

For now, I just have some drive-through comments about testing.

Also: would it make sense to have a JMH benchmark to prove that this code change is beneficial enough to warrant the additional complexity?

test/hotspot/jtreg/compiler/c2/TestSerialAdditions.java line 35:

> 33: /*
> 34:  * @test
> 35:  * @bug 8325495

Should we adjust / add the bug number?

test/hotspot/jtreg/compiler/c2/TestSerialAdditions.java line 291:

> 289:         return i + (x << i) + i; // Expects 64 + 63 + 64 = 191
> 290:     }
> 291: }

Would it make sense to add some randomized patterns, just for result verification?

You can use `Generators.java` to get interesting values. Of course that would mean not doing IR verification, but at least it would give us better confidence that the values are correct.

I'm imagining expressions like this:
`return a * CON1 + a * CON2 + a * CON3 + a * CON4`

Where the CON are defined as a `public static final` field with a random value generated by `Generators`.

The advantage of using `Generators` is that it generates powers-of-two more frequently, which seems to be relevant here.

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23506#pullrequestreview-2656414102
PR Review Comment: https://git.openjdk.org/jdk/pull/23506#discussion_r1978870944
PR Review Comment: https://git.openjdk.org/jdk/pull/23506#discussion_r1978880629