RFR: 8332268: C2: Add missing optimizations for UDivI/L and UModI/L and unify the shared logic with the signed nodes [v8]

Quan Anh Mai qamai at openjdk.org
Thu Nov 28 14:37:42 UTC 2024


On Thu, 28 Nov 2024 13:36:01 GMT, theoweidmannoracle <duke at openjdk.org> wrote:

>> This PR introduces
>> - several new optimizations to unsigned division and modulo
>>    - x % 1, x % x, x % 2^k
>>    - x / 1, x / x, x / 2^k 
>>    - does not implement the Granlund and Montgomery algorithm, which has been implemented for signed modulo division in the past. It is unclear if a lot is to be gained by implementing this.
>> - tests to test existing optimizations for signed division and modulo 
>>    - does not test the Granlund and Montgomery algorithm directly
>
> theoweidmannoracle has updated the pull request incrementally with one additional commit since the last revision:
> 
>   abs(MIN_INT) is not positive

test/hotspot/jtreg/compiler/c2/irTests/UDivINodeIdealizationTests.java line 88:

> 86: 
> 87:     @Test
> 88:     @IR(failOn = {IRNode.DIV})

All these should be `failOn = {IRNode.UDIV}`

test/hotspot/jtreg/compiler/c2/irTests/UDivINodeIdealizationTests.java line 129:

> 127:             IRNode.DIV_BY_ZERO_TRAP, "1"
> 128:     })
> 129:     // Hotspot should keep the division because it may cause a division by zero trap

This comment is actually incorrect, the division is kept because the transformation is wrong.

test/hotspot/jtreg/compiler/c2/irTests/UDivINodeIdealizationTests.java line 146:

> 144:     @IR(counts = {IRNode.URSHIFT, "1"})
> 145:     public int divByPow2Big(int x) {
> 146:         return Integer.divideUnsigned(x, -2147483648);  // -2147483648 = Integer.parseUnsignedInt("2147483648")

You should use `Integer.MIN_VALUE`

test/hotspot/jtreg/compiler/c2/irTests/UDivLNodeIdealizationTests.java line 146:

> 144:     @IR(counts = {IRNode.URSHIFT, "1"})
> 145:     public long divByPow2Big(long x) {
> 146:         return Long.divideUnsigned(x, -9223372036854775808L);  // -9223372036837998592 = Long.parseUnsignedLong("9223372036854775808")

Similarly, this should be `Long.MIN_VALUE` or `1 << 63`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22061#discussion_r1862298357
PR Review Comment: https://git.openjdk.org/jdk/pull/22061#discussion_r1862300217
PR Review Comment: https://git.openjdk.org/jdk/pull/22061#discussion_r1862301584
PR Review Comment: https://git.openjdk.org/jdk/pull/22061#discussion_r1862304464


More information about the hotspot-compiler-dev mailing list