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

[Emanuel Peter]

On Wed, 4 Sep 2024 08:42:26 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/rangeinference.cpp line 46:
>> 
>>> 44:   RangeInt<U> _bounds;
>>> 45:   KnownBits<U> _bits;
>>> 46: };
>> 
>> Could there be a `static_assert` for the `U` unsigned type?
>> 
>> A quick comment about the semantics of these classes would be appreciated.
>
> I think you could generally add `static_asserts` everywhere you use `U` or `S` types.

Yes, I definately need some explanation about what `present`, `progress`, `data` mean in these classes. It is not directly clear from the use-site to me, and I don't want to reverse-engineer to much as a reader ;)

>> src/hotspot/share/opto/rangeinference.cpp line 64:
>> 
>>> 62:       assert(zero_violation == 0, "");
>>> 63:       return lo;
>>> 64:     }
>> 
>> It would be nice if you state a definition `violation` in words in a comment.
>> 
>> 
>> lo     = 1100
>> zeros  = 1111
>> ones   = 1111
>> zv = 1100
>> ov = 0011
>> -> I struggle to see what exactly is violated here...
>> -> The "violations" are non-zero, so I'd assume the lo is outside what the bits allow... but that is not true.
>
> `one_violation`: bit that could be one, but is zero
> `zero_violation`: bit that could be zero, but is one
> Hmm. Do we assume that `KnownBits` is "sane", i.e. that we never have `zeros[i] = 0 = ones[i]`?
> Because then we are basically asserting that `ones = ~zeros = 0`, right? And so the bits only allow a single bit pattern.
> 
> I'm probably just very confused and need some more comments here.

I won't read more down before I understand this first part.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17508#discussion_r1743405601
PR Review Comment: https://git.openjdk.org/jdk/pull/17508#discussion_r1743403130