RFR: 8364407: [REDO] Consolidate Identity of self-inverse operations

Manuel Hässig mhaessig at openjdk.org
Mon Aug 18 16:56:14 UTC 2025


On Mon, 18 Aug 2025 15:49:48 GMT, Hannes Greule <hgreule at openjdk.org> wrote:

> The previous approach was flawed for `short` and `char` as these are int-subtypes and truncate the result (see the backout issue https://bugs.openjdk.org/browse/JDK-8364409 for a reproducer).
> 
> This change now first ensures that the input type is small enough so no truncation gets lost when dropping the operations.
> 
> The previous implementation also used an `InvolutionNode` superclass with one `Identity(...)` implementation, but there were some reservations whether this is the right way to go. As we now have a `ReverseBytesNode`, there is also less benefit in having the supertype, as this covers 4 in 1 already.
> 
> I also added test cases on top of the original ones that ensure the nodes stay when we can't prove the input type is small enough.

Thank you for noticing the bug and improving your original PR, @SirYwell! I like your new approach without a superclass and your extensive testing of all edge cases and random values. Nice work!

However, I do have some questions below.

src/hotspot/share/opto/subnode.cpp line 2080:

> 2078:     if (type == nullptr || involution->bottom_type()->is_int()->contains(type)) {
> 2079:       return involution->in(1)->in(1);
> 2080:     }

Instead of skipping the optimization, could you "clean" `involution->in(1)->in(1)` using `mask_int_value()`? That would follow the semantics of [JVMS§6.5 `ireturn`](https://docs.oracle.com/javase/specs/jvms/se24/html/jvms-6.html#jvms-6.5.ireturn).

test/hotspot/jtreg/compiler/c2/gvn/InvolutionIdentityTests.java line 1:

> 1: /*

Why are you not testing involution on `NegL/I` nodes? Can this not be optimized?

test/hotspot/jtreg/compiler/c2/gvn/InvolutionIdentityTests.java line 40:

> 38:  * @bug 8350988 8364407
> 39:  * @summary Test that Identity simplifications of Involution nodes are being performed as expected.
> 40:  * @library /test/lib /

Suggestion:

 * @summary Test that Identity simplifications of Involution nodes are being performed as expected.
 * @key randomness
 * @library /test/lib /

Since you are using random inputs.

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

Changes requested by mhaessig (Committer).

PR Review: https://git.openjdk.org/jdk/pull/26823#pullrequestreview-3129106380
PR Review Comment: https://git.openjdk.org/jdk/pull/26823#discussion_r2282919600
PR Review Comment: https://git.openjdk.org/jdk/pull/26823#discussion_r2282938964
PR Review Comment: https://git.openjdk.org/jdk/pull/26823#discussion_r2282897963


More information about the hotspot-compiler-dev mailing list