On Tue, 5 May 2026 16:51:59 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).
>
> Quan Anh Mai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Various fixes

Mostly cosmetic drive-by review.

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

> 442:   }
> 443: 
> 444:   // q = mul_hi(x, c) >> (s - 64) + (x < 0 ? 1 : 0) = mul_hi(x, c) >> (x 
> - 64) - (x >> 63)

`>> (x - 64) - (x >> 63)` -- the first one should be `(s - 64)`, correct?

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

> 1085: 
> //------------------------------Idealize---------------------------------------
> 1086: Node *UDivLNode::Ideal(PhaseGVN *phase, bool can_reshape) {
> 1087:   return unsigned_div_ideal<TypeLong, julong>(phase, can_reshape, this);

Is `unsigned_div_ideal` now dead?

test/hotspot/jtreg/compiler/c2/irTests/DivINodeIdealizationTests.java line 154:

> 152:     // Having a large enough and in the dividend removes the need to 
> account for rounding when converting to shifts and multiplies as in 
> divByPow2()
> 153:     public int divByPow2And(int x) {
> 154:         return (x & -6) / 2;

The comment above is now stale, as `-6` is not `2^c0`.

test/hotspot/jtreg/compiler/integerArithmetic/DivisionByConstant.java line 82:

> 80: 
> 81:     public static void main(String[] args) {
> 82:         Random r = Utils.getRandomInstance();

The test needs `@key randomness` to advertise it uses random.

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

PR Review: https://git.openjdk.org/jdk/pull/31033#pullrequestreview-4326199444
PR Review Comment: https://git.openjdk.org/jdk/pull/31033#discussion_r3272076179
PR Review Comment: https://git.openjdk.org/jdk/pull/31033#discussion_r3272063628
PR Review Comment: https://git.openjdk.org/jdk/pull/31033#discussion_r3272068197
PR Review Comment: https://git.openjdk.org/jdk/pull/31033#discussion_r3272060335

Reply via email to