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

[Roland Westrelin]

On Wed, 31 Aug 2022 03:36:26 GMT, Pengfei Li <pli at openjdk.org> wrote:

>> src/hotspot/share/opto/loopTransform.cpp line 2783:
>> 
>>> 2781:       }
>>> 2782:       if (p_short_scale != NULL) {
>>> 2783:         *p_short_scale = short_scale_l || short_scale_r;
>> 
>> I think it should be short_scale_l && short_scale_r here
>
>> I think it should be short_scale_l && short_scale_r here
> 
> Hi @rwestrel , may I ask you a question about this? From your comments, I see `short_scale` reports if a `ConvI2L` node is present since it's used to protect against overflow. Does this mean that `ConvI2L` at this point only appears in long counted loops? I ask this because in my knowledge array address computing in int loops also generates `ConvI2L` on 64-bit platforms.

It's for loops like this one:
    public static void testStridePosScalePosInIntLoop1(int start, int stop, long length, long offset) {
        final long scale = 2;
        final int stride = 1;

        // Same but with int loop
        for (int i = start; i < stop; i += stride) {
            Objects.checkIndex(scale * i + offset, length);
        }
    }

It's an int loop but because length is a long, there's an implicit cast of scale * i + offset to long (which is where the ConvI2L comes from). In the case of your change an expression for the range check that would need to be optimized would be:
((long)i) * scale
with scale 5 for instance so expressed by the compiler as ((long)i) << 2 + ((long)i) << 1
and both calls to is_scalled_iv would return true for short_scale which is why I think it should short_scale_l && short_scale_r
You're right that address computation includes a ConvI2L on 64 bits but the range check doesn't in:
array[i] = val;

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

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