* [PATCH 2/4] AArch64: Fix cost for Q register moves
@ 2014-09-04 14:45 Wilco Dijkstra
2014-09-04 15:34 ` Marcus Shawcroft
0 siblings, 1 reply; 5+ messages in thread
From: Wilco Dijkstra @ 2014-09-04 14:45 UTC (permalink / raw)
To: gcc-patches
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)
};
/* 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/4] AArch64: Fix cost for Q register moves
2014-09-04 14:45 [PATCH 2/4] AArch64: Fix cost for Q register moves Wilco Dijkstra
@ 2014-09-04 15:34 ` Marcus Shawcroft
2014-09-04 15:41 ` Wilco Dijkstra
0 siblings, 1 reply; 5+ messages in thread
From: Marcus Shawcroft @ 2014-09-04 15:34 UTC (permalink / raw)
To: Wilco Dijkstra; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 2/4] AArch64: Fix cost for Q register moves
2014-09-04 15:34 ` Marcus Shawcroft
@ 2014-09-04 15:41 ` Wilco Dijkstra
2014-09-04 15:53 ` Marcus Shawcroft
0 siblings, 1 reply; 5+ messages in thread
From: Wilco Dijkstra @ 2014-09-04 15:41 UTC (permalink / raw)
To: 'Marcus Shawcroft'; +Cc: gcc-patches
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/4] AArch64: Fix cost for Q register moves
2014-09-04 15:41 ` Wilco Dijkstra
@ 2014-09-04 15:53 ` Marcus Shawcroft
2014-09-11 15:14 ` Wilco Dijkstra
0 siblings, 1 reply; 5+ messages in thread
From: Marcus Shawcroft @ 2014-09-04 15:53 UTC (permalink / raw)
To: Wilco Dijkstra; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 2/4] AArch64: Fix cost for Q register moves
2014-09-04 15:53 ` Marcus Shawcroft
@ 2014-09-11 15:14 ` Wilco Dijkstra
0 siblings, 0 replies; 5+ messages in thread
From: Wilco Dijkstra @ 2014-09-11 15:14 UTC (permalink / raw)
To: 'Marcus Shawcroft'; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1454 bytes --]
Patch attached for commit as I don't have write access.
ChangeLog:
2014-09-11 Wilco Dijkstra <wdijkstr@arm.com>
* gcc/config/aarch64/aarch64.c (aarch64_register_move_cost):
Fix Q register move handling. (generic_regmove_cost): Undo raised
FP2FP move cost as Q register moves are now handled correctly.
> -----Original Message-----
> From: Marcus Shawcroft [mailto:marcus.shawcroft@gmail.com]
> Sent: 04 September 2014 16:54
> To: Wilco Dijkstra
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH 2/4] AArch64: Fix cost for Q register moves
>
> 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
[-- Attachment #2: Fix-Q-register-cost.txt --]
[-- Type: text/plain, Size: 1236 bytes --]
---
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 56b8eda..62b0168 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -215,10 +215,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)
};
/* Generic costs for vector insn classes. */
@@ -5961,7 +5958,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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-09-11 15:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-04 14:45 [PATCH 2/4] AArch64: Fix cost for Q register moves Wilco Dijkstra
2014-09-04 15:34 ` Marcus Shawcroft
2014-09-04 15:41 ` Wilco Dijkstra
2014-09-04 15:53 ` Marcus Shawcroft
2014-09-11 15:14 ` Wilco Dijkstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).