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

[Jasmine Karthikeyan]

On Fri, 14 Feb 2025 04:39:56 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 45 commits:
> 
>  - Merge branch 'openjdk:master' into xor_const
>  - fix variable names in comments
>  - update test
>  - address review comments
>  - formatting, remove commented tests
>  - add IR tests for long, simplify tests for int
>  - formatting
>  - add sanity asserts to tests
>  - re-add tests
>  - try fewer tests
>  - ... and 35 more: https://git.openjdk.org/jdk/compare/ff52859d...16049cdc

This looks really nice! I just have 2 small comments here.

src/hotspot/share/opto/addnode.cpp line 1032:

> 1030:   // round_up is safe because high bit is unset (0 <= lo <= hi)
> 1031: 
> 1032:   return round_up_power_of_2(U(hi_0 | hi_1) + 1) - 1 ;

Suggestion:

  return round_up_power_of_2(U(hi_0 | hi_1) + 1) - 1;

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

> 313: 
> 314:     @Test
> 315:     public int testXorConstRange(int x, int y) {

Should this have an `@IR` test attached, like `testFoldableXor`?

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

PR Review: https://git.openjdk.org/jdk/pull/23089#pullrequestreview-2621997896
PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1958977989
PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1958978363