Re: [PATCH]Construct canonical scaled address expression in IVOPT

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Sep 24, 2013 at 1:40 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Tue, Sep 24, 2013 at 6:12 PM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Tue, Sep 24, 2013 at 8:20 AM, bin.cheng <bin.cheng@arm.com> wrote:
>>>
>>>
>>>> -----Original Message-----
>>
>> Or even [reg*scale] (not sure about that).  But yes, at least reg*scale + offset
>> and reg*scale + reg.
>>
>>>   Apparently it's infeasible to check every
>>> possibility for each architecture, is it ok we at least check "index", then
>>> "addr" if "index" is failed?  By "any kind of addressing modes", I mean
>>> modes supported by function get_address_cost,  i.e., in form of "[base] +
>>> [off] + [var] + (reg|reg*scale)".
>>
>> I suppose so, but it of course depends on what IVOPTs uses the answer
>> for in the end.  Appearantly it doesn't distinguish between the various cases
>> even though TARGET_MEM_REF does support all the variants in question
>> (reg * scale, reg * scale + reg, reg * scale + const, reg * scale +
>> reg + const).
>>
>> So the better answer may be to teach the costs about the differences?
> Ideally, IVOPT should be aware whether scaling is allowed in every
> kind of addressing modes and account cost of multiplier accordingly.
> For current code, there are two scenarios here
> 1) If target supports reg*scale+reg, but not reg*scale, in this case,
> IVOPT considers multiplier is not allowed in any addressing mode and
> account multiplier with high cost.  This is the problem arm having.
> 2) If target supports reg*scale, but not some kind of addressing mode
> (saying reg*scale+reg), in this case, IVOPT still constructs various
> scaled addressing mode in get_address_cost and depends on address_cost
> to compute correct cost for that addressing expression.  I think this
> happens to work even IVOPT doesn't know "reg*scale+reg" is actually
> not supported.
>
>>
>>>> The above also builds more RTX waste which you can fix by re-using the
>>> PLUS
>>>> by building it up-front similar to the multiplication.  You also miss the
>>> Yes, this can be fixed.
>>>
>>>> opportunity to have scale == 1 denote as to whether reg1 + reg2 is valid.
>>> I
>>>> would expect that many targets support reg1 * scale + constant-offset but
>>>> not many reg1 * scale + reg2.
>>> I thought scale==1 is unnecessary because the addressing mode degrades into
>>> "reg" or "reg+reg".  Moreover, calls of multiplier_allowed_in_address_p in
>>> both get_address_cost and get_computation_cost_at have scale other than 1.
>>
>> Ok.
>>
>>>>
>>>> So no, the helper now checks sth completely different.  What's the problem
>>>> with arm supporting reg1 * scale?  Why shouldn't it being able to handle
>>> the
>>>> implicit zero offset?
>>>
>>> As Richard clarified, ARM does not support scaled addressing mode without
>>> base register.
>>
>> I see.
>>
> Also from the newer comments:
>
>> Btw, it should be reasonably possible to compute the whole
>> multiplier_allowed_in_address_p table for all primary and secondary archs
>> (simply build cross-cc1) and compare the results before / after a patch
>> candidate.  Querying both reg * scale and reg + reg * scale if the first
>> fails sounds like a good solution to me.
> I take this as we should do minimal change by checking reg + reg *
> scale if reg * scale is failed,  right?

Yes, you can share a single RTL expression for all this and I think
querying reg + reg * scale first makes sense (then fallback to reg * scale
for compatibility).

Richard.

> Thanks.
> bin
> --
> Best Regards.