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

[Emanuel Peter]

On Wed, 4 Sep 2024 08:41:07 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:
>> 
>>   move static_asserts
>
> 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.

> 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.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17508#discussion_r1743319474
PR Review Comment: https://git.openjdk.org/jdk/pull/17508#discussion_r1743400519