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

[Emanuel Peter]

On Wed, 26 Feb 2025 02:51:12 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.
>> 
>> ---------
>> ### Progress
>> - [x] Change must not contain extraneous whitespace
>> - [x] Commit message must refer to an issue
>> - [ ] Change must be properly reviewed (2 reviews required, with at least 2 [Reviewers](https://openjdk.org/bylaws#reviewer))
>> 
>> 
>> 
>> ### Reviewers
>>  * [Quan Anh Mai](https://openjdk.org/census#qamai) (@merykitty - Committer) 🔄 Re-review required (review applies to [cf779497](https://git.openjdk.org/jdk/pull/23089/files/cf77949776f7a4601268c7291a5743c2eb164186))
>> 
>> ### Reviewing
>> <details><summary>Using <code>git</code></summary>
>> 
>> Checkout this PR locally: \
>> `$ git fetch https://git.openjdk.org/jdk.git pull/23089/head:pull/23089` \
>> `$ git checkout pull/23089`
>> 
>> Update a local copy of the PR: \
>> `$ git checkout pull/23089` \
>> `$ git pull https://git.openjdk.org/jdk.git pull/23089/head`
>> 
>> </details>
>> <details><summary>Using Skara CLI tools</summary>
>> 
>> Checkout this PR locally: \
>> `$ git pr checkout 23089`
>> 
>> View PR using the GUI difftool: \
>> `$ git pr show -t 23089`
>> 
>> </details>
>> <details><summary>Using diff file</summary>
>> 
>> Download this PR as a diff file: \
>> <a href="https://git.openjdk.org/jdk/pull/23089.diff">https://git.openjdk.org/jdk/pull/23089.diff</a>
>> 
>> </details>
>> <details><summary>Using Webrev</summary>
>> 
>> [Link to Webrev Comment](https://git.openjdk.org/jdk/pull/23089#issuecomment-25939...
>
> Johannes Graham has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 51 commits:
> 
>  - Merge branch 'openjdk:master' into xor_const
>  - widen range of test values; add missing comment
>  - a few more tests
>  - add comments
>    
>    Co-authored-by: Emanuel Peter <emanuel.peter at oracle.com>
>  - update tests
>  - Fix formatting
>    
>    Co-authored-by: Jasmine Karthikeyan <25208576+jaskarth at users.noreply.github.com>
>  - Merge branch 'openjdk:master' into xor_const
>  - fix variable names in comments
>  - update test
>  - address review comments
>  - ... and 41 more: https://git.openjdk.org/jdk/compare/037e4711...6d60ae2a

test/hotspot/jtreg/compiler/c2/irTests/XorINodeIdealizationTests.java line 62:

> 60: 
> 61:         int min = Integer.MIN_VALUE;
> 62:         int max = MAX_VALUE;

Let's make it consistent, and either use `Integer.` everywhere or without it everywhere ;)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1971010016