RFR: 8291669: [REDO] Fix array range check hoisting for some scaled loop iv [v3]

[Tobias Hartmann]

On Fri, 2 Sep 2022 06:11:58 GMT, Pengfei Li <pli at openjdk.org> wrote:

>> This is a REDO of JDK-8289996. In previous patch, we defer some strength
>> reductions in Ideal functions of `Mul[I|L]Node` to post loop igvn phase
>> to fix a range check hoisting issue. More about previous patch can be
>> found in PR #9508, where we have described some details of the issue
>> we would like to fix.
>> 
>> Previous patch was backed out due to some jtreg failures found. We have
>> analyzed those failures one by one and found one of them exposes a real
>> performance regression. We see that deferring some strength reductions
>> to post loop igvn phase has too much impact. Some vector multiplication
>> will not be optimized to vector addition with vector shift after that
>> change. So in this REDO we propose the range check hoisting fix with a
>> different approach.
>> 
>> In this new patch, we add some recursive pattern matches for scaled loop
>> iv in function `PhaseIdealLoop::is_scaled_iv()`. These include matching
>> a sum or a difference of two scaled iv expressions. With this, all kinds
>> of Ideal-transformed scaled iv expressions can still be recognized. This
>> new approach only touches loop transformation code and hence has much
>> smaller impact. We have verified that this new approach applies to both
>> int range checks and long range checks.
>> 
>> Previously attached jtreg case fails on ppc64 because VectorAPI has no
>> vector intrinsics on ppc64 so there's no long range check to hoist. In
>> this patch, we limit the test architecture to x64 and AArch64.
>> 
>> Tested hotspot::hotspot_all_no_apps, jdk::tier1~3 and langtools::tier1.
>
> Pengfei Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update p_short_scale compuation

src/hotspot/share/opto/loopTransform.cpp line 2776:

> 2774:         // This logic is shared by int and long. For int, the result may overflow
> 2775:         // as we use jlong to compute so do the check here. Long result may also
> 2776:         // overflow but that's fine because result wraps.

But doesn't this mean that we bail out for integer overflows while not bailing out for long overflows?

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

PR: https://git.openjdk.org/jdk/pull/9851