RFR: 8350988: Consolidate Identity of self-inverse operations
Chen Liang
liach at openjdk.org
Mon Mar 3 03:29:59 UTC 2025
On Sat, 1 Mar 2025 13:41:31 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.
>
> test/hotspot/jtreg/compiler/c2/irTests/InvolutionIdentityTests.java line 118:
>
>> 116:
>> 117: @Test
>> 118: @IR(failOn = {IRNode.REVERSE_BYTES_I})
>
> I'm not sure if this is fine as the ReverseBytes nodes depend on intrinsics. From my understanding, the methods are just seen as normal methods on platforms without reverseBytes support. In that case, the test would still pass, but it might be surprising that it passes. Is this fine or is there a better way here?
Interesting question - expanding on that, could arbitrary methods be marked as self-inverse to be represented in the IR?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23851#discussion_r1976828917
More information about the hotspot-compiler-dev
mailing list