RFR: 8367341: C2: apply KnownBits and unsigned bounds to And / Or operations

[Emanuel Peter]

On Fri, 3 Oct 2025 06:07:50 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

> Hi,
> 
> This PR improves the implementation of `AndNode/OrNode/XorNode::Value` by taking advantages of the additional information in `TypeInt`. The implementation is pretty straightforward. A clever trick is that by analyzing the negative and positive ranges of a `TypeInt` separately, we have better info for the leading bits. I also implement gtest unit tests to verify the correctness and monotonicity of the inference functions.
> 
> Please take a look and leave your reviews, thanks a lot.

test/hotspot/gtest/opto/test_rangeinference.cpp line 33:

> 31: #include <limits>
> 32: #include <type_traits>
> 33: #include <unordered_set>

I don't know the current state of code style guide: but are we allowed to use `std::unordered_set`?

test/hotspot/gtest/opto/test_rangeinference.cpp line 250:

> 248:   static_assert(std::is_same_v<T, TypeIntMirror>);
> 249:   return *this;
> 250: }

We now re-implement these from `TypeIntHelper::int_type_xmeet`. I wonder if we could not at least share some code. Not sure if that is worth it. But having this kind of code duplication opens the risk of divergence and hence bugs.

test/hotspot/gtest/opto/test_rangeinference.cpp line 277:

> 275:     return 1732;
> 276:   }
> 277: }

What do the numbers mean here? I'm lost :/

test/hotspot/gtest/opto/test_rangeinference.cpp line 313:

> 311:     res[idx] = t;
> 312:     idx++;
> 313:   }

Not sure if this is possible with `std::array`, but you could do it with `std::vector`:
`std::vector tmp(unordered.begin(), unordered.end());`

Just an idea, feel free to leave it as is.

test/hotspot/gtest/opto/test_rangeinference.cpp line 327:

> 325: // on all elements of input1 and input2.
> 326: template <class InputType, class Operation, class Inference>
> 327: static void test_binary_instance_correctness_exhaustive(Operation op, Inference infer, const InputType& input1, const InputType& input2) {

Very nice!

test/hotspot/gtest/opto/test_rangeinference.cpp line 370:

> 368:       }
> 369:     }
> 370:   };

Using uniform distribution will make it very unlikely that you get a hit in a narrow long range, right?
Maybe we just have to live with that. Doing something smarter could probably be done (generate in the signed / unsigned bounds, and masking the bits), but there is also a risk: we may generate values that are too narrow by accident / bug...

What do you think? At least adding some comment here about why we do what we do would be good.

test/hotspot/gtest/opto/test_rangeinference.cpp line 449:

> 447:       if (all_instances<InputType>().size() < 100) {
> 448:         // This effectively covers the cases up to uintn_t<2>
> 449:         test_binary_instance_monotonicity_exhaustive(infer, input1, input2);

Wow, that's really not much. It's really only a "sign" bit and one "mantissa" bit. Would have been nice if we could have handled at least 3 bits. Is that prohibitively slow?

test/hotspot/gtest/opto/test_rangeinference.cpp line 524:

> 522:     samples[idx] = TypeIntMirror<S, U>{canonicalized_t._data._srange._lo, canonicalized_t._data._srange._hi,
> 523:                                        canonicalized_t._data._urange._lo, canonicalized_t._data._urange._hi,
> 524:                                        canonicalized_t._data._bits};

What about using a constructor that creates `TypeIntMirror` directly from a `TypeIntPrototype`? Maybe there is a reason that does not work?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27618#discussion_r2426329802
PR Review Comment: https://git.openjdk.org/jdk/pull/27618#discussion_r2426343721
PR Review Comment: https://git.openjdk.org/jdk/pull/27618#discussion_r2426350244
PR Review Comment: https://git.openjdk.org/jdk/pull/27618#discussion_r2426364709
PR Review Comment: https://git.openjdk.org/jdk/pull/27618#discussion_r2426373540
PR Review Comment: https://git.openjdk.org/jdk/pull/27618#discussion_r2426388502
PR Review Comment: https://git.openjdk.org/jdk/pull/27618#discussion_r2426401408
PR Review Comment: https://git.openjdk.org/jdk/pull/27618#discussion_r2426418049