[9] RFR(S): 8075136: Unnecessary sign extension for byte array access

[Vladimir Kozlov]

Looks good. What about other platforms? I see the same optimization in aarch64.ad

Thanks,
Vladimir

On 3/13/15 7:15 AM, Tobias Hartmann wrote:
> Hi,
>
> please review the following patch.
>
> https://bugs.openjdk.java.net/browse/JDK-8075136
> http://cr.openjdk.java.net/~thartmann/8075136/webrev.00/
>
> Problem:
> C2 adds an unnecessary 'movslq' sign extension while compiling a byte array access:
>
>    public static byte accessByte(int index) {
>      return byteArr[index];
>    }
>
>    0x00007f76f4747208: movslq %esi,%r11
>    0x00007f76f474720b: movsbl 0x10(%r10,%r11,1),%eax
>
> Where the 'movslq' is not necessary because we emit range checks guaranteeing that index %esi is not negative.
> For a char array access no such sign extension is created:
>
>    public static char accessChar(int index) {
>      return charArr[index];
>    }
>
>    0x00007fab3916b188: movzwl 0x10(%r10,%rsi,2),%eax
>
> This is because we only have matching rules to fold the corresponding ConvI2LNode in the following case:
>
>    match(AddP (AddP (DecodeN reg) (LShiftL (ConvI2L idx) scale)) off);
>
> This applies to charAccess [1] but not to byteAccess [2] because the byte array access does not require a scaling factor.
>
> Solution:
> I added the corresponding matching rule to fold the ConvI2LNode.
>
>    match(AddP (AddP (DecodeN reg) (ConvI2L idx)) off);
>
> Testing:
> - Testcase
> - JPRT
>
> Thanks,
> Tobias
>
> [1] https://bugs.openjdk.java.net/secure/attachment/26108/accessChar.png
> [2] https://bugs.openjdk.java.net/secure/attachment/26107/accessByte.png
>