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

[Pengfei Li]

On Thu, 25 Aug 2022 08:53:23 GMT, Roland Westrelin <roland 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.
>
> src/hotspot/share/opto/loopTransform.cpp line 2773:
> 
>> 2771:       // AddX(iv*K1, iv*K2) => iv*(K1+K2)
>> 2772:       jlong scale_sum = java_add(scale_l, scale_r);
>> 2773:       if (scale_sum > max_signed_integer(exp_bt) || scale_sum < min_signed_integer(exp_bt)) {
> 
> That doesn't look good to me. This logic is shared between int and long counted loops. For a long counted loop, scale_l and scale_r are longs and testing for overflow that way doesn't work.

My intention for this is to check INT overflow only. As this logic is shared by both INT and LONG, and we are using `java_add(jlong, jlong)` to add two INT scales, the addition result may only exceed the bounds of INT. For LONG scale values, it may overflow but that's fine because the result of `java_add()` wraps, so I don't think we need to check that for LONGs.

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

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