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

Emanuel Peter epeter at openjdk.org
Mon Dec 2 10:10:45 UTC 2024


On Mon, 2 Dec 2024 10:05:18 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> theoweidmannoracle has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix test
>
> src/hotspot/share/opto/divnode.cpp line 1156:
> 
>> 1154:   }
>> 1155:   // Don't bother trying to transform a dead node
>> 1156:   if (mod->in(0) && mod->in(0)->is_top()) {
> 
> Suggestion:
> 
>   if (mod->in(0) != nullptr && mod->in(0)->is_top()) {
> 
> 
> https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md
> `Do not use ints or pointers as (implicit) booleans with &&, ||, if, while. Instead, compare explicitly, i.e. if (x != 0) or if (ptr != nullptr), etc.`

There are more instances in the code. I know you copied it, but when we touch code we make sure to adhere to the new rules ;)

> test/hotspot/jtreg/compiler/c2/irTests/ModINodeIdealizationTests.java line 43:
> 
>> 41: 
>> 42:     public static void main(String[] args) {
>> 43:         TestFramework.runWithFlags("-XX:CompileCommand=inline,*Math::max");
> 
> Is this really necessary? Should this not happen automatically?

If you do need it: can you add a comment why?

> test/hotspot/jtreg/compiler/c2/irTests/UDivLNodeIdealizationTests.java line 120:
> 
>> 118:     // Checks x / (c / c) => x
>> 119:     public long identityAgainButBig(long x) {
>> 120:         return Long.divideUnsigned(x, Long.divideUnsigned(-9214294468834361176L, -9214294468834361176L));  // Long.parseUnsignedLong("9232449604875190440") = -9214294468834361176L
> 
> What is the comment for here?

Is this `Long.MIN_VALUE`? Can you explain more in the comment, and move it on a separate line?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22061#discussion_r1865569534
PR Review Comment: https://git.openjdk.org/jdk/pull/22061#discussion_r1865553451
PR Review Comment: https://git.openjdk.org/jdk/pull/22061#discussion_r1865565224


More information about the hotspot-compiler-dev mailing list