RFR: 8335444: Generalize implementation of AndNode mul_ring
Damon Fenacci
dfenacci at openjdk.org
Wed Jul 31 17:15:30 UTC 2024
On Wed, 31 Jul 2024 15:53:55 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:
>> src/hotspot/share/opto/mulnode.cpp line 638:
>>
>>> 636:
>>> 637: // Since the result of bitwise-and can be as high as the largest value of each range, the result should find the maximum.
>>> 638: return IntegerType::make(std::numeric_limits<NativeType>::min() >> shift_bits, MAX2(r0->_hi, r1->_hi), widen);
>>
>> I was just wondering if it would be OK to use `(r0->_lo & r1->_lo)` as the minimum value instead of finding the number of leading 1s and shifting. It seems to be a bit more constrained and compact (and seemingly correct). What do you think?
>
> I actually did experiment with this before coming up with the current approach, but I found that there are cases where it produces incorrect bounds. With a small example where `r0 = [-3, -1]` and `r1 = [-7, -1]`, using `r0->_lo & r1->_lo` results in `-3 & -7`, which equals `-7`. However, in the case where the value in `r0` is `-3` and `r1` is `-6`, we can get an out of bounds result as `-3 & -6` equals `-8`. Because of that I thought the best way to fix this was to find the common leading 1s. This approach does lead to us losing some information in the lower-order bits but I thought it was an acceptable compromise since the code to handle finding common bits across a range of integers becomes quite a bit more complicated. I hope this clarifies!
You're totally right! It is not even related to the LSB (`-14 & -6` would have the same problem with `-12`). Finding the leading 1s is the right solution. Thanks a lot for the clarification!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20066#discussion_r1698847781
More information about the hotspot-compiler-dev
mailing list