RFR: 8335444: Generalize implementation of AndNode mul_ring [v3]

[Jasmine Karthikeyan]

On Mon, 19 Aug 2024 12:06:16 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Jasmine Karthikeyan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Check IR before macro expansion
>
> src/hotspot/share/opto/mulnode.cpp line 635:
> 
>> 633:   // Since count_leading_zeros is undefined at 0 (~(-1)) the number of digits in the native type can be used instead,
>> 634:   // as it returns 31 and 63 for signed integers and longs respectively.
>> 635:   int shift_bits = sel_val == 0 ? std::numeric_limits<NativeType>::digits : count_leading_zeros(sel_val) - 1;
> 
> `sel_val` can only be 0 if `r0->_lo` and `r1->_lo` are both -1. While I think it's correct how you handle the case here, wouldn't it be simpler/more readable if you handle this case separately by setting -1 as lower bound directly instead of using "`min >> #digits`"?

This is a good point! The logic becomes a lot easier to parse when we don't need to work around `count_leading_zeros` being undefined at 0 and just set the lower bound manually.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20066#discussion_r1727781384