Re: [PATCH][expmed] Calculate mult-by-const cost properly in mult_by_coeff_cost

[Jeff Law <law@redhat.com>]

On 04/20/2015 03:27 AM, Kyrill Tkachov wrote:
>
> On 15/04/15 16:41, Jeff Law wrote:
>> On 04/14/2015 02:07 AM, Kyrill Tkachov wrote:
>>> Hi Jeff,
>>>
>>> Thanks for looking at this.
>>>
>>> On 13/04/15 19:18, Jeff Law wrote:
>>>> On 03/16/2015 04:12 AM, Kyrill Tkachov wrote:
>>>>> Hi all,
>>>>>
>>>>> Eyeballing the mult_by_coeff_cost function I think it has a
>>>>> typo/bug. It's supposed to return the cost of multiplying by
>>>>> a constant 'coeff'. It calculates that by taking the cost of
>>>>> a MULT rtx by that constant and comparing it to the cost of
>>>>> synthesizing that multiplication, and returning the cheapest.
>>>>> However, in the MULT rtx cost calculations it creates a MULT
>>>>> rtx of two REGs rather than the a REG and the GEN_INT of
>>>>> coeff as I would expect. This patches fixes that in the
>>>>> obvious way.
>>>>>
>>>>> Tested aarch64-none-elf and bootstrapped on
>>>>> x86_64-linux-gnu. I'm guessing this is stage 1 material at
>>>>> this point?
>>>>>
>>>>> Thanks, Kyrill
>>>>>
>>>>> 2015-03-13  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>>>
>>>>> * expmed.c (mult_by_coeff_cost): Pass CONT_INT rtx to MULT
>>>>> cost calculation rather than fake_reg.
>>>> I'm pretty sure this patch is wrong.
>>>>
>>>> The call you're referring to is computing an upper limit to the
>>>> cost for use by choose_mult_variant.  Once a synthesized
>>>> multiply sequence exceeds the cost of reg*reg, then that
>>>> synthesized sequence can be thrown away because it's not
>>>> profitable.
>>> But shouldn't the limit be the mult-by-constant cost?
>> No, because ultimately we're trying to do better than just loading
>> the constant into a register and doing a reg * reg.  So the reg*reg
>> case is the upper bound for allowed cost of a synthesized
>> sequence.
>
> So I've thought about it a bit more and I have another concern. The
> function returns this: if (choose_mult_variant (mode, coeff,
> &algorithm, &variant, max_cost)) return algorithm.cost.cost; else
> return max_cost;
>
> If I read this right, it tries to synthesise the mult at
> choose_mult_variant with the limit cost of the reg-by-reg mult, but
> if the synthesis cost exceeds that, then it returns the reg-by-reg
> mult cost (in return max_cost;) so that can't be right, can it?
In the case where the target doesn't have mult imm,reg, then reg*reg 
would be the right estimated cost if there's no cheap synthesis.   It 
doesn't look like we correctly handle costing on targets with mult imm,reg.

jeff