RFR(M): 8154826: AArch64: take better advantage of base + shifted offset addressing mode

[Vladimir Kozlov]

You did not remove Matcher::clone_shift_expressions variable initialization from all .ad files.

Changes look correct - you cloned shared code into .ad files and make code platform specific.
But duplicated code will increase maintenance cost. Can you leave code for clone_shift_expressions=false case (same code on several platforms) in matcher.cpp?

Thanks,
Vladimir

On 5/4/16 12:38 AM, Roland Westrelin wrote:
> Anyone for a review of this? It's aarch64 specific but has some shared
> code changes.
>
> Roland.
>
>
> On 04/25/2016 03:50 PM, Roland Westrelin wrote:
>>
>>
>> On 04/21/2016 10:23 AM, Roland Westrelin wrote:
>>> http://cr.openjdk.java.net/~roland/8154826/webrev.00/
>>>
>>> The aarch64 port implicitly transforms:
>>> (AddP base (AddP base address (LShiftL index con)) offset)
>>> into:
>>> (AddP base (AddP base offset) (LShiftL index con))
>>> in the ad file to embed the shift (and possibly and i2l conversion) into
>>> the addressing mode of a memory operation. Exposing that transformation
>>> in the ideal graph allows:
>>>
>>> - (AddP base offset) to be scheduled (for instance outside a loop)
>>> - multiple identical (AddP base offset) to be commoned
>>> - (LShiftL index con) to be cloned during matching so that each memory
>>> access has its own
>>
>> Further testing revealed some problems with the previous change so here
>> is a new webrev:
>>
>> http://cr.openjdk.java.net/~roland/8154826/webrev.01/
>>
>> Memory access instructions only accept a shift that matches the size of
>> the data being accessed (i.e. 2 for a 4 byte load). It's not always the
>> case that address expressions produced by c2 follow that constraint. As
>> expected, tt's very rare that it doesn't and seem to occur only with
>> unsafe. I added a predicate that skips the transformation of the address
>> subtree at the end of the optimization passes and prevent matching of
>> the address subtree if the constraint is not met for one use. As this is
>> uncommon, that seems good enough.
>>
>> Roland.
>>