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

Christian Hagedorn chagedorn at openjdk.org
Tue Dec 10 09:23:45 UTC 2024


On Tue, 10 Dec 2024 08:54:16 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 two additional commits since the last revision:
> 
>  - Fix space
>  - Add missing UDiv/UMod cases to no_dependent_zero_check and cannot_split_division

Thanks for fixing the cases! Some minor variable renaming suggestions for the existing code that you moved. Otherwise, looks good to me, too.

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

> 463:     return nullptr;
> 464:   }
> 465:   const TypeClass* tl = t->cast<TypeClass>();

I guess `tl` was for type long before which is no longer true. How about renaming this to `type_divisor` to make it more explicit? Then you can also rename `l` -> `divisor` further down.

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

> 1137:     return nullptr;
> 1138:   }
> 1139:   const TypeClass* ti = t->cast<TypeClass>();

Same here, you could also name this `type_divisor` and `con` -> `divisor`.

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

> 1194:   }
> 1195: 
> 1196:   const TypeClass* i1 = t1->cast<TypeClass>();

Could be applied here as well:
`i1` -> `type_dividend`
`i2` -> `type_divisor`

test/hotspot/jtreg/compiler/splitif/TestSplitDivisionThroughPhi.java line 78:

> 76:     public static void main(String[] strArr) {
> 77:         Integer.divideUnsigned(2, 3);
> 78:         Long.divideUnsigned(2, 3);

Maybe add a comment here:
Suggestion:

        // Make sure classes are loaded when compiling with -Xcomp
        Integer.divideUnsigned(2, 3);
        Long.divideUnsigned(2, 3);

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22061#pullrequestreview-2491523629
PR Review Comment: https://git.openjdk.org/jdk/pull/22061#discussion_r1877646189
PR Review Comment: https://git.openjdk.org/jdk/pull/22061#discussion_r1877658415
PR Review Comment: https://git.openjdk.org/jdk/pull/22061#discussion_r1877674222
PR Review Comment: https://git.openjdk.org/jdk/pull/22061#discussion_r1877667349


More information about the hotspot-compiler-dev mailing list