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