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