RFR: 8347645: C2: XOR bounded value handling blocks constant folding [v4]

Quan Anh Mai qamai at openjdk.org
Thu Jan 23 08:56:52 UTC 2025


On Thu, 23 Jan 2025 08:13:30 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Johannes Graham has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix test names
>
> src/hotspot/share/opto/addnode.cpp line 994:
> 
>> 992:   const TypeInt *r1 = t1->is_int();
>> 993: 
>> 994:   if( !r0->is_con() || !r1->is_con() ) {
> 
> Suggestion:
> 
>   if( !r0->is_con() || !r1->is_con()) {
> 
> That is the new style, please apply it everywhere in the code.

@eme64 I think you meant `if (!r0->is_con() || !r1->is_con())` :)

@j3graham I would prefer

    if (r0->is_con() && r1->is_con()) {
        return ...;
    }

It conveys the idea that we check for constant folding chance first in a clearer manner, it also enhances the aesthetic of the following code :)

> src/hotspot/share/opto/addnode.cpp line 1008:
> 
>> 1006:       const TypeInt* t2x = TypeInt::make(0, round_down_power_of_2(r1->_hi) + (round_down_power_of_2(r1->_hi) - 1), r1->_widen);
>> 1007:       return t1x->meet(t2x);
>> 1008:     }
> 
> I know you only moved this code - but can you add some more comments about what it does? Maybe add some examples? It would help me review this code faster if I don't have to figure it out myself ;)

Tbh I think this would be clearer, there is also no need for rounding down since the max value of `r->_hi` has the highest bit unset:

    // x ^ y cannot have any bit set that is higher than both the highest bits set in x and y
    // x cannot have any bit set that is higher than the highest bit set in r0->_hi
    // y cannot have any bit set that is higher than the highest bit set in r1->_hi
    juint max = round_up_power_of_2<juint>(r0->_hi | r1->_hi) - 1;
    return TypeInt::make(0, max, MAX2(r0->_widen, r1->_widen));

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1926572807
PR Review Comment: https://git.openjdk.org/jdk/pull/23089#discussion_r1926593715


More information about the hotspot-compiler-dev mailing list