RFR: 8364970: Redo JDK-8327381 by updating the CmpU type instead of the Bool type

[Christian Hagedorn]

On Wed, 6 Aug 2025 23:33:23 GMT, Francisco Ferrari Bihurriet <fferrari at openjdk.org> wrote:

> Hi, this pull request is a second take of 1383fec41756322bf2832c55633e46395b937b40, by updating the `CmpUNode` type as either `TypeInt::CC_LE` (case 1a) or `TypeInt::CC_LT` (case 1b) instead of updating the `BoolNode` type as `TypeInt::ONE`.
> 
> With this approach a56cd371a2c497e4323756f8b8a08a0bba059bf2 becomes unnecessary. Additionally, having the right type in `CmpUNode` could potentially enable further optimizations.
> 
> #### Testing
> 
> In order to evaluate the changes, the following testing has been performed:
> 
> * `jdk:tier1` (see [GitHub Actions run](https://github.com/franferrax/jdk/actions/runs/16789994433))
> * [`TestBoolNodeGVN.java`](https://github.com/openjdk/jdk/blob/jdk-26+9/test/hotspot/jtreg/compiler/c2/gvn/TestBoolNodeGVN.java), created for [JDK-8327381: Refactor type-improving transformations in BoolNode::Ideal to BoolNode::Value](https://bugs.openjdk.org/browse/JDK-8327381) (1383fec41756322bf2832c55633e46395b937b40)
>     * I also checked it breaks if I remove the `CmpUNode::Value_cmpu_and_mask` call
> * Private reproducer for [JDK-8349584: Improve compiler processing](https://bugs.openjdk.org/browse/JDK-8349584) (a56cd371a2c497e4323756f8b8a08a0bba059bf2)
> * A local slowdebug run of the `test/hotspot/jtreg/compiler/c2` category on _Fedora Linux x86_64_
>     * Same results as with `master` (f95af744b07a9ec87e2507b3d584cbcddc827bbd)

Thanks for improving this! I have some small suggestions, otherwise, it looks good to me!

> Additionally, having the right type in CmpUNode could potentially enable further optimizations.

Could you already find some examples, where this change gives us an improved IR? If so, you could also add it as IR test.

I'll also give this a spinning in our testing.

src/hotspot/share/opto/phaseX.cpp line 2941:

> 2939: //       Bool
> 2940: //
> 2941: void PhaseCCP::push_bool_with_cmpu_and_mask(Unique_Node_List& worklist, const Node* use) const {

Needed to double-check but I think it's fine to remove the notification code since we already have `push_cmpu()` in place which looks through the `AddI`:
https://github.com/openjdk/jdk/blob/10762d408bba9ce0945100847a8674e7eb7fa75e/src/hotspot/share/opto/phaseX.cpp#L2911-L2926

So, whenever `m` or `1` changes, we will re-add the `CmpU` to the CCP worklist with `push_cmpu()`. The `x` does not matter for the application of `Value_cmpu_and_mask()`.

src/hotspot/share/opto/subnode.cpp line 855:

> 853: // (1a) and (1b) is covered by this method since we can directly return the corresponding TypeInt::CC_*
> 854: // while (2) is covered in BoolNode::Ideal since we create a new non-constant node (see [CMPU_MASK]).
> 855: const Type* CmpUNode::Value_cmpu_and_mask(PhaseValues* phase, const Node* in1, const Node* in2) {

I suggest to directly name these:
`in1` -> `andI`
`in2`- > `rhs`

Then it's easier to follow the comments.

src/hotspot/share/opto/subnode.cpp line 1899:

> 1897: // based on local information.   If the input is constant, do it.
> 1898: const Type* BoolNode::Value(PhaseGVN* phase) const {
> 1899:   return _test.cc2logical( phase->type( in(1) ) );

Suggestion:

  return _test.cc2logical(phase->type(in(1)));

src/hotspot/share/opto/subnode.hpp line 174:

> 172: // Compare 2 unsigned values (integer or pointer), returning condition codes (-1, 0 or 1).
> 173: class CmpUNode : public CmpNode {
> 174:   static const Type* Value_cmpu_and_mask(PhaseValues*, const Node*, const Node*);

We usually add matching parameter names as found in the source file:

  static const Type* Value_cmpu_and_mask(PhaseValues* phase, const Node* in1, const Node* in2);

or with the renaming above:

  static const Type* Value_cmpu_and_mask(PhaseValues* phase, const Node* andI, const Node* rhs);

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/26666#pullrequestreview-3104636268
PR Review Comment: https://git.openjdk.org/jdk/pull/26666#discussion_r2266033683
PR Review Comment: https://git.openjdk.org/jdk/pull/26666#discussion_r2265997538
PR Review Comment: https://git.openjdk.org/jdk/pull/26666#discussion_r2266009017
PR Review Comment: https://git.openjdk.org/jdk/pull/26666#discussion_r2266038306