RFR: 8325495: C2: implement optimization for series of Add of unique value [v2]

[Roland Westrelin]

On Wed, 18 Sep 2024 20:39:06 GMT, Kangcheng Xu <kxu at openjdk.org> wrote:

>> src/hotspot/share/opto/addnode.cpp line 427:
>> 
>>> 425:   }
>>> 426: 
>>> 427:   Node* con = (bt == T_INT) ? (Node*) phase->intcon((jint) factor) : (Node*) phase->longcon(factor);
>> 
>> You can use `integercon()` and pass `bt`
>
> I disagree: `integercon()` internally uses `checked_cast<jint>(l)` to make prevent information loss during type conversion and asserts at runtime if the value is larger what a `jint` can hold. However, such an information loss is intended for integer arithmetic overflows. (e.g., `Integer.MAX_VALUE * a + a` is extracted to `((jlong) Integer.MAX_VALUE + (jlong) 1) * a`. Here we want `Integer.MAX_VALUE + 1` to overflow to `(int) Integer.MIN_VALUE`).
> 
> If I were to use `integercon()`, the best I could do is `intgercon(bt == T_INT ? (jint) factor : factor)` which is rather pointless.

Good catch.
It makes sense to add a comment about that in the source code.
Do you have a test case for that corner case?

>> src/hotspot/share/opto/addnode.cpp line 490:
>> 
>>> 488:     if (bt == T_INT || bt == T_LONG) { // const could potentially be void type
>>> 489:       Node* mul_base;
>>> 490:       jlong multiplier = extract_base_operand_from_serial_additions(phase, operand_node, &mul_base, depth_limit - 1);
>> 
>> Do you need to recurse at all here?
>
> I believe so. Consider the case `(a + a) * 3`. Recurse here allows us to extract `a` and factor `2 * 3 => 6`

That case would be better handled in 2 steps, I think:
`a+a` into `a*2` with a `AddNode` transformation
`(a*2)*3` into `a*6` with a `MulNode` (or `LShift`) transformation. Can you check if it already exists?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20754#discussion_r1771244410
PR Review Comment: https://git.openjdk.org/jdk/pull/20754#discussion_r1771246595