Re: [PATCH][AArch64] Improve aarch64_modes_tieable_p

[James Greenhalgh]

On Fri, Apr 22, 2016 at 01:22:51PM +, Wilco Dijkstra wrote:
> Improve modes_tieable by returning true in more cases: allow scalar access
> within vectors without requiring an extra move. Removing these moves helps
> the register allocator in deciding whether to use integer or FP registers on
> operations that can be done on both. This saves about 100 instructions in the
> gcc.target/aarch64 tests.
> 
> A typical example:
> 
>   orr v1.8b, v0.8b, v1.8b
>   fmovx0, d0
>   fmovx1, d1
>   add x0, x1, x0
>   ins v0.d[0], x0
>   orr v0.8b, v1.8b, v0.8b
> 
> after:
> 
>   orr v1.8b, v0.8b, v1.8b
>   add d0, d1, d0
>   orr v0.8b, v1.8b, v0.8b
> 
> OK for trunk?

OK.

Thanks,
James

> 
> ChangeLog:
> 2016-04-22  Wilco Dijkstra  
> 
>   * gcc/config/aarch64/aarch64.c (aarch64_modes_tieable_p):
>   Allow scalar/single vector modes to be tieable.
> 
 

Re: [PATCH][AArch64] Improve aarch64_modes_tieable_p

[Wilco Dijkstra]

ping


From: Wilco Dijkstra
Sent: 17 May 2016 17:08
To: James Greenhalgh
Cc: gcc-patches@gcc.gnu.org; nd
Subject: Re: [PATCH][AArch64] Improve aarch64_modes_tieable_p

James Greenhalgh wrote:
> It would be handy if you could raise something in bugzilla for the
> register allocator deficiency.

The register allocation issues are well known and we have multiple
workarounds for this in place. When you allow modes to be tieable
the workarounds are not as effective.

> -  if (TARGET_SIMD
> -  && aarch64_vector_mode_p (mode1)
> -  && aarch64_vector_mode_p (mode2))
> +  if (aarch64_vector_mode_p (mode1) && aarch64_vector_mode_p (mode2))
> +return true;

> This relaxes the TARGET_SIMD check that would have prevented
> OImode/CImode/XImode ties when !TARGET_SIMD. What's the reasoning
> behind that?

There is no need for TARGET_SIMD checks here - in order to create a
vector_struct mode you need to call aarch64_array_mode_supported_p first.

> +  /* Also allow any scalar modes with vectors.  */
> +  if (aarch64_vector_mode_supported_p (mode1)
> +  || aarch64_vector_mode_supported_p (mode2))
>  return true;

> Does this always hold? It seems like you might need to be more restrictive
> with what we allow to avoid ties with some of the more obscure modes
> (V4DF etc.).

Well it is safe to always return true - this passes regression tests (it's just 
a bad
idea from a CQ point of view).

Wilco

Re: [PATCH][AArch64] Improve aarch64_modes_tieable_p

[Wilco Dijkstra]

James Greenhalgh wrote:
> It would be handy if you could raise something in bugzilla for the
> register allocator deficiency.

The register allocation issues are well known and we have multiple
workarounds for this in place. When you allow modes to be tieable
the workarounds are not as effective.

> -  if (TARGET_SIMD
> -  && aarch64_vector_mode_p (mode1)
> -  && aarch64_vector_mode_p (mode2))
> +  if (aarch64_vector_mode_p (mode1) && aarch64_vector_mode_p (mode2))
> +return true;

> This relaxes the TARGET_SIMD check that would have prevented
> OImode/CImode/XImode ties when !TARGET_SIMD. What's the reasoning
> behind that?

There is no need for TARGET_SIMD checks here - in order to create a
vector_struct mode you need to call aarch64_array_mode_supported_p first.

> +  /* Also allow any scalar modes with vectors.  */
> +  if (aarch64_vector_mode_supported_p (mode1)
> +  || aarch64_vector_mode_supported_p (mode2))
>  return true;

> Does this always hold? It seems like you might need to be more restrictive
> with what we allow to avoid ties with some of the more obscure modes
> (V4DF etc.).

Well it is safe to always return true - this passes regression tests (it's just 
a bad
idea from a CQ point of view).

Wilco

Re: [PATCH][AArch64] Improve aarch64_modes_tieable_p

[James Greenhalgh]

On Fri, Apr 22, 2016 at 01:22:51PM +, Wilco Dijkstra wrote:
> Improve modes_tieable by returning true in more cases: allow scalar access
> within vectors without requiring an extra move. Removing these moves helps
> the register allocator in deciding whether to use integer or FP registers on
> operations that can be done on both. This saves about 100 instructions in the
> gcc.target/aarch64 tests.
>

[snip]

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index abc864c..6e921f0 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -12294,7 +12294,14 @@ aarch64_reverse_mask (enum machine_mode mode)
>return force_reg (V16QImode, mask);
>  }
>  
> -/* Implement MODES_TIEABLE_P.  */
> +/* Implement MODES_TIEABLE_P.  In principle we should always return true.
> +   However due to issues with register allocation it is preferable to avoid
> +   tieing integer scalar and FP scalar modes.  Executing integer operations
> +   in general registers is better than treating them as scalar vector
> +   operations.  This reduces latency and avoids redundant int<->FP moves.
> +   So tie modes if they are either the same class, or vector modes with
> +   other vector modes, vector structs or any scalar mode.
> +*/

*/ shouldn't be on the newline, just "[...] scalar mode.  */"

It would be handy if you could raise something in bugzilla for the
register allocator deficiency.

>  bool
>  aarch64_modes_tieable_p (machine_mode mode1, machine_mode mode2)
> @@ -12305,9 +12312,12 @@ aarch64_modes_tieable_p (machine_mode mode1, 
> machine_mode mode2)
>/* We specifically want to allow elements of "structure" modes to
>   be tieable to the structure.  This more general condition allows
>   other rarer situations too.  */
> -  if (TARGET_SIMD
> -  && aarch64_vector_mode_p (mode1)
> -  && aarch64_vector_mode_p (mode2))
> +  if (aarch64_vector_mode_p (mode1) && aarch64_vector_mode_p (mode2))
> +return true;

This relaxes the TARGET_SIMD check that would have prevented
OImode/CImode/XImode ties when !TARGET_SIMD. What's the reasoning
behind that?

> +  /* Also allow any scalar modes with vectors.  */
> +  if (aarch64_vector_mode_supported_p (mode1)
> +  || aarch64_vector_mode_supported_p (mode2))
>  return true;

Does this always hold? It seems like you might need to be more restrictive
with what we allow to avoid ties with some of the more obscure modes
(V4DF etc.).

Thanks,
James

[PATCH][AArch64] Improve aarch64_modes_tieable_p

[Wilco Dijkstra]

Improve modes_tieable by returning true in more cases: allow scalar access
within vectors without requiring an extra move. Removing these moves helps
the register allocator in deciding whether to use integer or FP registers on
operations that can be done on both. This saves about 100 instructions in the
gcc.target/aarch64 tests.

A typical example:

orr v1.8b, v0.8b, v1.8b
fmovx0, d0
fmovx1, d1
add x0, x1, x0
ins v0.d[0], x0
orr v0.8b, v1.8b, v0.8b

after:

orr v1.8b, v0.8b, v1.8b
add d0, d1, d0
orr v0.8b, v1.8b, v0.8b

OK for trunk?

ChangeLog:
2016-04-22  Wilco Dijkstra  

* gcc/config/aarch64/aarch64.c (aarch64_modes_tieable_p):
Allow scalar/single vector modes to be tieable.

---
 gcc/config/aarch64/aarch64.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index abc864c..6e921f0 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -12294,7 +12294,14 @@ aarch64_reverse_mask (enum machine_mode mode)
   return force_reg (V16QImode, mask);
 }
 
-/* Implement MODES_TIEABLE_P.  */
+/* Implement MODES_TIEABLE_P.  In principle we should always return true.
+   However due to issues with register allocation it is preferable to avoid
+   tieing integer scalar and FP scalar modes.  Executing integer operations
+   in general registers is better than treating them as scalar vector
+   operations.  This reduces latency and avoids redundant int<->FP moves.
+   So tie modes if they are either the same class, or vector modes with
+   other vector modes, vector structs or any scalar mode.
+*/
 
 bool
 aarch64_modes_tieable_p (machine_mode mode1, machine_mode mode2)
@@ -12305,9 +12312,12 @@ aarch64_modes_tieable_p (machine_mode mode1, 
machine_mode mode2)
   /* We specifically want to allow elements of "structure" modes to
  be tieable to the structure.  This more general condition allows
  other rarer situations too.  */
-  if (TARGET_SIMD
-  && aarch64_vector_mode_p (mode1)
-  && aarch64_vector_mode_p (mode2))
+  if (aarch64_vector_mode_p (mode1) && aarch64_vector_mode_p (mode2))
+return true;
+
+  /* Also allow any scalar modes with vectors.  */
+  if (aarch64_vector_mode_supported_p (mode1)
+  || aarch64_vector_mode_supported_p (mode2))
 return true;
 
   return false;
-- 
1.9.1