RFR: 8347645: C2: XOR bounded value handling blocks constant folding [v46]

[Vladimir Ivanov]

On Sun, 30 Mar 2025 03:14:32 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 incrementally with one additional commit since the last revision:
> 
>   add missing import

Thanks.

> 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.

There's usually a lot of invariants a function assumes and it's simply impractical to encode everything in the name. Speaking of this particular case (`calc_xor_upper_bound_of_non_neg`): 
  * `calc_` is redundant and IMO only adds noise;
  * `_non_neg` part is confusing; I'd stress instead that it works on **ranges**.

So, `xor_upper_bound_for_ranges` then? (And, please, explain in the comment what's the correspondense between `S` and `U` template type parameters.)

> `addnodeXorUtil.hpp`

I'm fine with placing it under `opto`. Please, rename the file into `src/hotspot/share/opto/utilities/xor.hpp`.

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

PR Comment: https://git.openjdk.org/jdk/pull/23089#issuecomment-2767559589