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