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

Emanuel Peter epeter at openjdk.org
Tue Sep 3 14:26:32 UTC 2024


On Tue, 3 Sep 2024 13:57:45 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Quan Anh Mai has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits:
>> 
>>  - fix compile errors
>>  - Merge branch 'master' into unsignedbounds
>>  - add comments
>>  - Merge branch 'master' into unsignedbounds
>>  - fix release build
>>  - add comments, group arguments to reduce C-style reference passing arguments
>>  - fix tests, add verify
>>  - add unit tests
>>  - fix template parameter
>>  - refactor
>>  - ... and 1 more: https://git.openjdk.org/jdk/compare/d8e4d3f2...d5ad9f1a
>
> src/hotspot/share/opto/rangeinference.cpp line 109:
> 
>> 107: static AdjustResult<KnownBits<T>>
>> 108: adjust_bits_from_bounds(const KnownBits<T>& bits, const RangeInt<T>& bounds) {
>> 109:   static_assert(std::is_unsigned<T>::value, "");
> 
> Again: could be a member function of `KnownBits`, where unsigned could be verified already...

At any rate, I would name `T -> U`

> src/hotspot/share/opto/type.hpp line 604:
> 
>> 602:   const jint _lo, _hi;       // Lower bound, upper bound in the signed domain
>> 603:   const juint _ulo, _uhi;    // Lower bound, upper bound in the unsigned domain
>> 604:   const juint _zeros, _ones; // Bits that are known to be 0 or 1
> 
> It would have been nice to have some sort of `Range` and `KnownBits` class... not sure how feasible this is, especially since `_lo` and `_hi` are already used all over the place...

Can you explain the semantics of the combination of these? Each of these defines a subset of the whole int-range. Is the resulting type the intersection of all of these three?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17508#discussion_r1742115104
PR Review Comment: https://git.openjdk.org/jdk/pull/17508#discussion_r1742074617


More information about the hotspot-compiler-dev mailing list