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

[Quan Anh Mai]

On Mon, 13 Oct 2025 13:30:39 GMT, Emanuel Peter <epeter 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 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.

`std::array` is trivially destructible, which is strongly encouraged for static objects in Hotspot.

> 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.

That is right, we can partially avoid this by exercising the random tests with narrow integral types. For wide types such as `jint` or ``jlong`, it is however unclear which distribution would be the best.

> 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?

Yes, testing the monotonicity for those is really slow since we traverse the set of all instances multiple times.

> 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?

It is only done here so it is questionable whether making another constructor is beneficial. This is also a testing backdoor since we don't want to create a `TypeIntMirror` with arbitrary field values.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27618#discussion_r2431661840
PR Review Comment: https://git.openjdk.org/jdk/pull/27618#discussion_r2431671085
PR Review Comment: https://git.openjdk.org/jdk/pull/27618#discussion_r2431673278
PR Review Comment: https://git.openjdk.org/jdk/pull/27618#discussion_r2431679428