[2/4] AArch64: Fix cost for Q register moves

This patch fixes a bug in aarch64_register_move_cost(): GET_MODE_SIZE is in bytes not bits. As a
result the FP2FP cost doesn't need to be set to 4 to catch the special case for Q register moves.

ChangeLog:
2014-09-04  Wilco Dijkstra  <wdijkstr@arm.com>

	* gcc/config/aarch64/aarch64.c (aarch64_register_move_cost):
	Fix Q register handling. Set FP2FP move cost.

---
 gcc/config/aarch64/aarch64.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

On 4 September 2014 15:45, Wilco Dijkstra <wdijkstr@arm.com> wrote:
> This patch fixes a bug in aarch64_register_move_cost(): GET_MODE_SIZE is in bytes not bits. As a
> result the FP2FP cost doesn't need to be set to 4 to catch the special case for Q register moves.
>
> ChangeLog:
> 2014-09-04  Wilco Dijkstra  <wdijkstr@arm.com>
>
>         * gcc/config/aarch64/aarch64.c (aarch64_register_move_cost):
>         Fix Q register handling. Set FP2FP move cost.
>
> ---
>  gcc/config/aarch64/aarch64.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 6245f59..57bb083 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -219,10 +219,7 @@ static const struct cpu_regmove_cost generic_regmove_cost =
>    NAMED_PARAM (GP2GP, 1),
>    NAMED_PARAM (GP2FP, 2),
>    NAMED_PARAM (FP2GP, 2),
> -  /* We currently do not provide direct support for TFmode Q->Q move.
> -     Therefore we need to raise the cost above 2 in order to have
> -     reload handle the situation.  */
> -  NAMED_PARAM (FP2FP, 4)
> +  NAMED_PARAM (FP2FP, 2)

This is not directly related to the change below and it is missing
from the ChangeLog.   Originally this number had to be > 2 in order
for secondary reload to kick in.  See the comment above the second
hunk of this patch.  Why is it OK to lower this number ?

>  };
>
>  /* Generic costs for vector insn classes.  */
> @@ -5846,7 +5843,7 @@ aarch64_register_move_cost (enum machine_mode mode,
>       secondary reload.  A general register is used as a scratch to move
>       the upper DI value and the lower DI value is moved directly,
>       hence the cost is the sum of three moves. */
> -  if (! TARGET_SIMD && GET_MODE_SIZE (mode) == 128)
> +  if (! TARGET_SIMD && GET_MODE_SIZE (mode) == 16)

This hunk is OK and it matches the ChangeLog.

>      return regmove_cost->GP2FP + regmove_cost->FP2GP + regmove_cost->FP2FP;
>
>    return regmove_cost->FP2FP;
> --
> 1.9.1
>
>

I think it best to split this patch into the two different parts, you
can consider the second part pre-approved.

Cheers
/Marcus

> From: Marcus Shawcroft [mailto:marcus.shawcroft@gmail.com]
> > -  NAMED_PARAM (FP2FP, 4)
> > +  NAMED_PARAM (FP2FP, 2)
> 
> This is not directly related to the change below and it is missing
> from the ChangeLog.   Originally this number had to be > 2 in order
> for secondary reload to kick in.  See the comment above the second
> hunk of this patch.  Why is it OK to lower this number ?

It is related because the GET_MODE_SIZE bug means it never returns the
correct cost, but instead returns the FP2FP cost. So the FP2FP cost had
to be artificially increased. With the fix this is no longer required.

> >  /* Generic costs for vector insn classes.  */
> > @@ -5846,7 +5843,7 @@ aarch64_register_move_cost (enum machine_mode mode,
> >       secondary reload.  A general register is used as a scratch to move
> >       the upper DI value and the lower DI value is moved directly,
> >       hence the cost is the sum of three moves. */
> > -  if (! TARGET_SIMD && GET_MODE_SIZE (mode) == 128)
> > +  if (! TARGET_SIMD && GET_MODE_SIZE (mode) == 16)
> >      return regmove_cost->GP2FP + regmove_cost->FP2GP + regmove_cost->FP2FP;
> >
> >    return regmove_cost->FP2FP;
> > --
> > 1.9.1

On 4 September 2014 16:41, Wilco Dijkstra <wdijkstr@arm.com> wrote:
>> From: Marcus Shawcroft [mailto:marcus.shawcroft@gmail.com]
>> > -  NAMED_PARAM (FP2FP, 4)
>> > +  NAMED_PARAM (FP2FP, 2)
>>
>> This is not directly related to the change below and it is missing
>> from the ChangeLog.   Originally this number had to be > 2 in order
>> for secondary reload to kick in.  See the comment above the second
>> hunk of this patch.  Why is it OK to lower this number ?
>
> It is related because the GET_MODE_SIZE bug means it never returns the
> correct cost, but instead returns the FP2FP cost. So the FP2FP cost had
> to be artificially increased. With the fix this is no longer required.

Yep, I read the code again, I understand.  You still need to fix the
ChangeLog.  OK to commit with a fixed ChangeLog.

Cheers

/Marcus