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

[Andrew Haley]

On 03/18/2015 12:42 PM, Tobias Hartmann wrote:
> 
> On 18.03.2015 12:38, Andrew Haley wrote:
>> On 03/18/2015 11:19 AM, Tobias Hartmann wrote:
>>>
>>> On 18.03.2015 11:27, Andrew Haley wrote:
>>>> On 03/18/2015 09:51 AM, Tobias Hartmann wrote:
>>>>> Byte array access:
>>>>>   0x0000007f7453d8b4: add	x10, x10, w1, sxtw
>>>>>   0x0000007f7453d8b8: ldrsb	w0, [x10,#16]
>>>>>
>>>>> Char array access:
>>>>>   0x0000007f7cb6f8b4: add	x8, x10, #0x10
>>>>>   0x0000007f7cb6f8b8: ldrh	w0, [x8,w1,sxtw #1]
>>>>
>>>> Do you know why these are different?  The second form is definitely
>>>> preferable.
>>>
>>> Yes, it's because we have no operand for the indirect-with-offset
>>> (no scaling) case and therefore it is matched to 'addP_reg_reg_ext'.
>>>
>>> For this reason I added the two new operands indIndexOffsetI2L and
>>> indIndexOffsetI2LN. But even with those operands, it only works if
>>> we reduce their cost below INSN_COST. Otherwise, 'AddP (AddP reg
>>> (ConvI2L ireg)) off' is matched to 'addP_reg_reg_ext' (cost
>>> INSN_COST)
>>
>> Aha!  That's the bug.  'addP_reg_reg_ext' should be 2*INSN_COST,
>> along with all the ALU extend and/or shift instructions.  My
>> bad, probably.
> 
> Okay, I assume you meant '1.9*INSN_COST' as it is used for 'addP_reg_reg_lsl'?

Yes indeed, you're right again.  :-)

> Here is the updated webrev:
> http://cr.openjdk.java.net/~thartmann/8075136/webrev.05/
> 
> It works fine:
>   0x0000007f6d1c4a34: add	x8, x10, #0x10
>   0x0000007f6d1c4a38: ldrsb	w0, [x8,w1,sxtw #0]

Excellent.

Thanks,
Andrew.