RFR: 8327381 Refactor type-improving transformations in BoolNode::Ideal to BoolNode::Value [v4]

[Emanuel Peter]

On Wed, 13 Mar 2024 02:05:39 GMT, Kangcheng Xu <kxu at openjdk.org> wrote:

>> This PR resolves [JDK-8327381](https://bugs.openjdk.org/browse/JDK-8327381)
>> 
>> Currently the transformations for expressions with patterns `((x & m) u<= m)` or `((m & x) u<= m)` to `true` is in `BoolNode::Ideal` function with a new constant node of value `1` created. However, this is technically a type-improving (reduction in range) transformation that's better suited in `BoolNode::Value` function.
>> 
>> New unit test `test/hotspot/jtreg/compiler/c2/TestBoolNodeGvn.java` asserting on IR nodes and correctness of this transformation is added and passing.
>
> Kangcheng Xu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   update the package name for tests

The cpp code looks good, I have some small issues with the test. But we are close ;)

test/hotspot/jtreg/compiler/c2/irTests/TestBoolNodeGvn.java line 41:

> 39:  * @run driver compiler.c2.irTests.TestBoolNodeGvn
> 40:  */
> 41: public class TestBoolNodeGvn {

Suggestion:

public class TestBoolNodeGVN {

I would have used the capital for GVN, since it is an acronym, and we use it capitalized everywhere.

test/hotspot/jtreg/compiler/c2/irTests/TestBoolNodeGvn.java line 52:

> 50:      */
> 51:     @Test
> 52:     @Arguments({Argument.DEFAULT, Argument.DEFAULT})

You will have to merge from master, and fix this line. There was a change in the `@Arguments` annotation recently.
Suggestion:

    @Arguments(values = {Argument.DEFAULT, Argument.DEFAULT})


I got lots of build issues:

    @Arguments({Argument.DEFAULT, Argument.DEFAULT})
               ^
  symbol:   method value()
  location: @interface Arguments

test/hotspot/jtreg/compiler/c2/irTests/TestBoolNodeGvn.java line 58:

> 56:                 & !(Integer.compareUnsigned((m & x), m) > 0)
> 57:                 & Integer.compareUnsigned((x & m), m + 1) < 0
> 58:                 & Integer.compareUnsigned((m & x), m + 1) < 0;

For easier reading, I would have put the `&` at the end of the line.
Btw: is this supposed to be a bitwise or a binary and?

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18198#pullrequestreview-1948764568
PR Review Comment: https://git.openjdk.org/jdk/pull/18198#discussion_r1532014803
PR Review Comment: https://git.openjdk.org/jdk/pull/18198#discussion_r1532009274
PR Review Comment: https://git.openjdk.org/jdk/pull/18198#discussion_r1532009265