RFR: 8364766: C2: Improve Value() of DivI and DivL for non-constant inputs [v2]

[Tobias Hotz]

On Thu, 7 Aug 2025 09:26:40 GMT, Manuel Hässig <mhaessig at openjdk.org> wrote:

>> Tobias Hotz has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fixes after review
>
> src/hotspot/share/opto/divnode.cpp line 568:
> 
>> 566:   if (i2_lo != i2_hi) {
>> 567:     // special case not possible here, _lo mus
>> 568:     assert(i2_lo != -1, "Special case not possible here");
> 
> While functionally correct, here you are only talking about the negative special case, but if `i2_lo in [0,1]` the same might happen on the positive side.
> Suggestion:
> 
>   // If the divisor range is wider than a singleton, include (i1->_lo, i2->_lo).
>   // We cannot use is_con here, as a range of [-1, 0] for i2_hi and [0,1] for i2_lo
>   // will also result in i2_lo and i2_hi being -1, or i2_lo and i2_hi being 1
>   // respectively.
>   if (i2_lo != i2_hi) {
>     assert(i2_hi - i2_lo >= 1, "i2 must be wider that a singleton");

You are right, because for hi, it doesn't matter and would just be a useless computation, see my comment below. We need to make sure i2_lo is != -1 to avoid running into UB.

> src/hotspot/share/opto/divnode.cpp line 575:
> 
>> 573: 
>> 574:   // If i1 is not a single constant, include the two corners with i1->_hi:
>> 575:   //   (i1->_hi, i2->_lo) and (i1->_hi, i2->_hi)
> 
> Why do you not have to handle the case of `i2` being a singleton range?

Also the same reason: i2 being a singleton is no problem, we just do some useless calculations, but we can not end up in UB due to i1 being > min_int

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26143#discussion_r2265263302
PR Review Comment: https://git.openjdk.org/jdk/pull/26143#discussion_r2265263541