RFR: 8350988: Consolidate Identity of self-inverse operations
Emanuel Peter
epeter at openjdk.org
Wed Mar 12 12:39:04 UTC 2025
On Sat, 1 Mar 2025 13:34:30 GMT, Hannes Greule <hgreule at openjdk.org> wrote:
> subnode has multiple nodes that are self-inverse but lacking the respective optimization. ReverseINode and ReverseLNode already have the optimization, but we can deduplicate the code for all those operations.
>
> For most nodes, the optimization is obvious. The NegF/DNodes however are worth to look at in detail imo:
> - `Float.NaN` has the same bits set as `-Float.NaN`. That means, it this specific case, the operation is a no-op anyway
> - For other values, the msb is flipped, flipping twice results in the original value again.
>
> Similar changes could be made to the corresponding vector nodes. If you want, I can do that in a follow-up RFE.
>
> One note: During benchmarking those changes, I ran into https://bugs.openjdk.org/browse/JDK-8307516. That means code like
>
> int v = 0;
> for (int datum : data) {
> v ^= Integer.reverseBytes(Integer.reverseBytes(datum));
> }
> return v;
>
> was vectorized before but is considered "not profitable" with the changes here, causing slowdowns in such cases.
Nice idea! Thanks for the work :)
test/hotspot/jtreg/compiler/c2/irTests/InvolutionIdentityTests.java line 83:
> 81: assertResultF(nanf);
> 82:
> 83: double ad = RunInfo.getRandom().nextDouble();
This actually only generates values between `0.0...1.0`.
Can you instead use `Generators.java`? It will make sure to generate "interesting" values, including different encodings of `NaN`, infinity, etc.
-------------
Changes requested by epeter (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/23851#pullrequestreview-2678314776
PR Review Comment: https://git.openjdk.org/jdk/pull/23851#discussion_r1991404835
More information about the hotspot-compiler-dev
mailing list