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