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

Quan Anh Mai qamai at openjdk.org
Fri May 2 11:28:22 UTC 2025


On Fri, 2 May 2025 08:35:44 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/utilities/intn_t.hpp line 58:
> 
>> 56:   }
>> 57: 
>> 58:   constexpr static int min = std::numeric_limits<unsigned int>::max() << (n - 1);
> 
> Ok, so the lower `n-1` bits are zero, and the uppermost is `1`. Why not just shift up a `1`? Or do you actually care about the upper bits? What exactly is the general assumption about the upper bits?

This is an `int`, not an `intn_t<nbits>`, so we need all the upper bits be 1, too.

> src/hotspot/share/utilities/intn_t.hpp line 134:
> 
>> 132: };
>> 133: 
>> 134: }
> 
> Could use indentation to make clear where the namespace starts and ends. Or at least a comment like this:
> Suggestion:
> 
> namespace std {
> 
> template <unsigned int n>
> class numeric_limits<intn_t<n>> {
> public:
>   constexpr static intn_t<n> min() { return intn_t<n>(intn_t<n>::min); }
>   constexpr static intn_t<n> max() { return intn_t<n>(intn_t<n>::max); }
> };
> 
> template <unsigned int n>
> class numeric_limits<uintn_t<n>> {
> public:
>   constexpr static uintn_t<n> min() { return uintn_t<n>(uintn_t<n>::min); }
>   constexpr static uintn_t<n> max() { return uintn_t<n>(uintn_t<n>::max); }
> };
> 
> } // namespace std

Added the comment marking the end of the namespace. We often do not indent for namespace in C++ I believe.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17508#discussion_r2071472663
PR Review Comment: https://git.openjdk.org/jdk/pull/17508#discussion_r2071473294


More information about the hotspot-compiler-dev mailing list