On Tue, 5 May 2026 05:32:20 GMT, Quan Anh Mai <[email protected]> wrote:
> Hi, > > I was reminded of this forgotten PR when reviewing a counted loop > transformation PR. The important point is that it is easier and more > efficient to compute the trip count of a counted loop using unsigned > division. Currently, for int counted loops, trip count is computed by > extending the loop parameters to long and doing a signed long division. This > cannot be applied to long counted loop. As a result, as a precondition for > long counted loop predication, we need to be able to efficiently transform an > unsigned division by constant. > > For more information, please refer to #9947 . > > Testing: > > - [x] tier1-4,hs-comp-stress > > Please take a look and leave your review, thanks a lot. > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). Few comments. I did not verify your math - testing should catch any issue. Thank you for extending testing. src/hotspot/share/opto/divnode.cpp line 99: > 97: } > 98: > 99: // magic_divide_constants in divconstants.cpp calculates the constant c, s "the constant c, s" - what is 's'? src/hotspot/share/opto/divnode.cpp line 116: > 114: bool d_pos = divisor >= 0; > 115: juint d = g_uabs(divisor); > 116: constexpr int N = 32; Where `N` is used? src/hotspot/share/opto/divnode.cpp line 211: > 209: static Node* transform_int_udivide(PhaseGVN* phase, Node* dividend, > juint divisor) { > 210: assert(divisor > 1, "invalid constant divisor"); > 211: constexpr int N = 32; Can you put `N = 32;` here near it use? src/hotspot/share/opto/divnode.cpp line 536: > 534: if (n->in(0) != nullptr && n->in(0)->is_top()) { > 535: return nullptr; > 536: } `remove_dead_region()` should remove this node if its control is top. May be use assert here. Or don't check at all. test/hotspot/jtreg/compiler/c2/cr7200264/TestIntVect.java line 519: > 517: > 518: @Test > 519: @IR(counts = { IRNode.RSHIFT_VI, Why this change? test/hotspot/jtreg/compiler/c2/irTests/DivINodeIdealizationTests.java line 88: > 86: Asserts.assertEQ(a / (13 / 13), identityAgain(a)); > 87: Asserts.assertEQ(a / -1, divByNegOne(a)); > 88: Asserts.assertEQ((a & -6) / 2, divByPow2And(a)); It was `(a & -4)` before. Why change? Because it was 0 and you want result not 0? ------------- PR Review: https://git.openjdk.org/jdk/pull/31033#pullrequestreview-4229576559 PR Review Comment: https://git.openjdk.org/jdk/pull/31033#discussion_r3189812725 PR Review Comment: https://git.openjdk.org/jdk/pull/31033#discussion_r3189832456 PR Review Comment: https://git.openjdk.org/jdk/pull/31033#discussion_r3189869699 PR Review Comment: https://git.openjdk.org/jdk/pull/31033#discussion_r3189940138 PR Review Comment: https://git.openjdk.org/jdk/pull/31033#discussion_r3189800394 PR Review Comment: https://git.openjdk.org/jdk/pull/31033#discussion_r3189757216
