RFR: 8364766: C2: Improve Value() of DivI and DivL for non-constant inputs [v10]

[Emanuel Peter]

On Sun, 9 Nov 2025 09:34:52 GMT, Tobias Hotz <duke at openjdk.org> wrote:

>> This PR improves the value of interger division nodes.
>> Currently, we only emit a good type if either input is constant. But we can also cover the generic case. It does that by finding the four corners of the division. This is guranteed to find the extrema that we can use for min/max. Some special logic is required for MIN_INT / -1, though, as this is a special case
>> We also need some special logic to handle ranges that cross zero, but in this case, we just need to check for the negative and positive range once.
>> This also cleans up and unifies the code paths for DivINode and DivLNode.
>> I've added some tests to validate the optimization. Without the changes, some of these tests fail.
>
> Tobias Hotz has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Move Test to compiler.igvn

Very nice work @ichttt ! I like all your comments, and thanks for all the test cases, including the randomized ones!

I just have a few minor suggestions :)

src/hotspot/share/opto/divnode.cpp line 566:

> 564:     } else {
> 565:       new_hi = min_val;
> 566:     }

Suggestion:

    if (!i1->is_con()) {
      // a) non-constant dividend: i1 could be min_val + 1.
      // -> i1 / i2 = (min_val + 1) / -1 = max_val is possible.
      new_hi = max_val;
      assert((min_val + 1) / -1 == new_hi, "new_hi should be max_val");
    } else if (i2_lo != i2_hi) {
      // b) i1 is constant min_val, i2 is non-constant.
      //    if i2 = -1 -> i1 / i2 =  min_val / -1 = min_val
      //    if i2 < -1 -> i1 / i2 <= min_val / -2 = (max_val / 2) + 1
      new_hi = (max_val / 2) + 1;
      assert(min_val / -2 == new_hi, "new_hi should be (max_val / 2) + 1)");
    } else {
      // c) i1 is constant min_val, i2 is constant -1.
      //    -> i1 / i2 = min_val / -1 = min_val
      new_hi = min_val;
    }

Reading this, I felt like I had to reconstruct a lot in my head. We could help the reader a little to close the gap.
Feel free to reformulate, and even to remove you list above in favour of the inlined comments.

test/hotspot/jtreg/compiler/igvn/IntegerDivValueTests.java line 66:

> 64:         // All constants available during parsing
> 65:         return getIntConstant(Integer.MIN_VALUE) / getIntConstant(-1);
> 66:     }

Why not add an IR rule that the div is still present after parsing? It seems you have already had the possible issue that javac optimized the div away, right? So this would ensure the optimization really does happen in C2, and that you are checking for the right kinds of nodes.

test/hotspot/jtreg/compiler/igvn/IntegerDivValueTests.java line 70:

> 68:     @Test
> 69:     @IR(failOn = {IRNode.DIV_I, IRNode.DIV_L, IRNode.URSHIFT_I, IRNode.URSHIFT_L, IRNode.RSHIFT_I, IRNode.RSHIFT_L, IRNode.MUL_I, IRNode.MUL_L, IRNode.ADD_I, IRNode.ADD_L, IRNode.SUB_I, IRNode.SUB_L, IRNode.AND_I, IRNode.AND_L})
> 70: 

Suggestion:

test/hotspot/jtreg/compiler/igvn/IntegerDivValueTests.java line 286:

> 284:     // transform_long_divide splits up the division into multiple other nodes, such as MulHiLNode, which does not have a good Value() implemantion.
> 285:     // When JDK-8366815 is fixed, these rules should be reenabled
> 286:     // Alternatively, a better MulHiLNode::Value() implemantion should also lead to constant folding

Could you have some temporary IR rule that now passes, but fails once `JDK-8366815` is fixed? Otherwise, I'm afraid we will miss these comments here, and they will never be cleaned up.

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

PR Review: https://git.openjdk.org/jdk/pull/26143#pullrequestreview-3459442625
PR Review Comment: https://git.openjdk.org/jdk/pull/26143#discussion_r2523188178
PR Review Comment: https://git.openjdk.org/jdk/pull/26143#discussion_r2523208681
PR Review Comment: https://git.openjdk.org/jdk/pull/26143#discussion_r2523209322
PR Review Comment: https://git.openjdk.org/jdk/pull/26143#discussion_r2523214237