RFR: 8347645: C2: XOR bounded value handling blocks constant folding [v45]
Johannes Graham
duke at openjdk.org
Sun Mar 30 02:48:41 UTC 2025
On Sun, 30 Mar 2025 02:39:27 GMT, Johannes Graham <duke at openjdk.org> wrote:
>> An interaction between xor bounds optimization and constant folding resulted in xor over constants not being optimized. This has a noticeable effect on `Long.expand` with a constant mask, on architectures that don't have instructions equivalent to `PDEP` to be used in an intrinsic.
>>
>> This change moves logic from the `Xor(L|I)Node::Value` methods into the `add_ring` methods, and gives priority to constant-folding. A static method was separated out to facilitate direct unit-testing. It also (subjectively) simplified the calculation of the upper bound and added an explanation of the reasoning behind it.
>>
>> In addition to testing for constant folding over xor, IR tests were added to `XorINodeIdealizationTests` and `XorLNodeIdealizationTests` to cover these related items:
>> - Bounds optimization of xor
>> - A check for `x ^ x = 0`
>> - Explicit testing of xor over booleans.
>>
>> Also `test_xor_node.cpp` was added to more extensively test the correctness of the bounds optimization. It exhaustively tests ranges of 4-bit numbers as well as at the high and low end of the affected types.
>
> Johannes Graham has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 61 commits:
>
> - Merge branch 'openjdk:master' into xor_const
> - move code into a header file that can be shared with GTEST
> - Undo accidental changes to Int tests
> - Add random range tests for Long
> - Merge branch 'openjdk:master' into xor_const
> - Merge branch 'openjdk:master' into xor_const
> - invert comparison in tests
> - update bug numbers and summary
> - add test of random ranges
> - consistency
> - ... and 51 more: https://git.openjdk.org/jdk/compare/3d2c3cd4...ce17608b
The naming of that method evolved during the course of the review of this PR. I believe the thinking was that the check was not necessarily an overall upper bound, and a simpler name would imply it was more general.
I have separated the logic out into its own header file, but I've put it under `opto`. `utilities` felt too far away from where it was used. I've called it `addnodeXorUtil.hpp` with the thinking that it could become the home for more code - it's a pretty small piece of code to have its own header. Other naming suggestions would be welcomed.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/23089#issuecomment-2764352131
More information about the hotspot-compiler-dev
mailing list