RFR: 8373999: C2: apply KnownBits and unsigned bounds to Add / Sub operations [v4]

Quan Anh Mai qamai at openjdk.org
Wed Jan 14 13:39:40 UTC 2026


On Wed, 14 Jan 2026 09:51:06 GMT, Benoît Maillard <bmaillard 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 incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>> 
>>  - copyright year
>>  - Merge branch 'master' into addsub
>>  - Merge branch 'master' into addsub
>>  - include order
>>  - Improve Add/SubNode::Value with unsigned bounds and known bits
>
> src/hotspot/share/opto/rangeinference.hpp line 453:
> 
>> 451:       //   sum[i] = bit & 1;
>> 452:       //   carry[i - 1] = (bit >= 2);
>> 453:       // }
> 
> Is there a specific reason why the notation here goes from `n-1` to `0` and not the reverse? I find it more intuitive to have index `0` for the least significant bit, but maybe there is some convention I am not aware of. It does not matter too much in any case, so feel free to do whatever.

Since we are viewing the binary number as a bit string, I tend to think that it is more intuitive to imagine a value 0b1011 as a bit string "1011", which means the first index is the msb and the last index is the lsb. Normally, when I do Maths I use 0 as the lsb, though, since numbers are unbounded. But for this presentation, I think doing it this way is easier for the others to conceptualize.

> src/hotspot/share/opto/rangeinference.hpp line 480:
> 
>> 478:       //
>> 479:       // If we gather the min_bits into a value tmp, it is clear that
>> 480:       // tmp = st1._bits._ones + st2._bits._ones:
> 
> It feels like we don't need to "initialize" `tmp`, but maybe I am missing something

This is not an initialization, though. This section just describes that a `tmp` constructed using the loop below is the same as the one constructed by adding these 2 values.

> src/hotspot/share/opto/rangeinference.hpp line 491:
> 
>> 489:       // min_bit >= 2 if and only if either:
>> 490:       // - st1._bits._ones[i] == st2._bits._ones[i] == 1
>> 491:       // - (st1._bits._ones[i] == 1 || st2._bits._ones[i] == 1) && ((min_bit & 1) == 0)
> 
> If I am not mistaken we could also write it this way, and I personally find this a bit more intuitive (and also more consistent with the subtraction case). And for the subsequent computations we could replace `|` by `^`. 
> Suggestion:
> 
>       // - (st1._bits._ones[i] != st2._bits._ones[i]) && ((min_bit & 1) == 0)

Yes you are right, that's a good idea.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28897#discussion_r2690456483
PR Review Comment: https://git.openjdk.org/jdk/pull/28897#discussion_r2690461242
PR Review Comment: https://git.openjdk.org/jdk/pull/28897#discussion_r2690457238


More information about the hotspot-compiler-dev mailing list