RFR: 8315066: Add unsigned bounds and known bits to TypeInt/Long [v56]

[Quan Anh Mai]

On Fri, 2 May 2025 07:06:24 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Emanuel's reviews
>
> src/hotspot/share/opto/type.hpp line 293:
> 
>> 291: 
>> 292:   template <typename TypeClass>
>> 293:   const TypeClass* try_cast() const;
> 
> This is a way to get to the `isa_...` via templated types, right?
> 
> I wonder if it might be better to name it `isa_???`, or even just `isa`, so that it is clearer that it is about the `isa` query.
> 
> Currently, it is not very clear what it does, until you look at the implementation. That's a bit unfortunate.

I like the naming `try_cast` better because it aligns with the semantics of `std::dynamic_cast`. `isa` is a bad name.

> src/hotspot/share/opto/type.hpp line 735:
> 
>> 733:  *   b. t._hi < 0. Similarly, t._lo == jint(t._ulo) and t._hi == jint(t._uhi)
>> 734:  *
>> 735:  *   c. t._lo < 0, t._hi >= 0.
> 
> Suggestion:
> 
>  *   c. t._lo < 0, 0 <= t._hi.
> 
> I like ordering numbers according to their value :)

Personally I like having a named variable in the lhs and a constant in the rhs, it also makes the case distinction clearer. You have either `t._lo < 0` or `t._lo >= 0`.

> src/hotspot/share/opto/type.hpp line 772:
> 
>> 770: private:
>> 771:   TypeInt(const TypeIntPrototype<jint, juint>& t, int w, bool dual);
>> 772:   static const Type* try_make(const TypeIntPrototype<jint, juint>& t, int widen, bool dual);
> 
> Just an idea, very optional.
> `try_make` does not say what it does when it fails.
> Exception? `nullptr`? `TOP`?
> You you could rename it to `try_make_else_null` or `make_or_null`. Something like that.

Yah `make_or_top` seems to be a good name.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17508#discussion_r2071305594
PR Review Comment: https://git.openjdk.org/jdk/pull/17508#discussion_r2071303369
PR Review Comment: https://git.openjdk.org/jdk/pull/17508#discussion_r2071302242