[PATCH][AArch64] Improve spill code - swap order in shr patterns

2015-07-27 Thread Wilco Dijkstra
This is basically the same as the shl version but for the shr patterns.

Various instructions are supported as integer operations as well as SIMD on 
AArch64. When
register pressure is high, lra-constraints inserts spill code without taking 
the allocation
class into account, and basically chooses the first available pattern that 
matches. Since this
instruction has the SIMD version first it is usually chosen eventhough some of 
the operands
are eventually allocated to integer registers. The result is inefficient code 
not only due to
the higher latency of SIMD instructions but also due to the extra int<->FP 
moves. Placing the
integer variant first in the shr pattern generates far more optimal spill code.

2015-07-27  Wilco Dijkstra  

* gcc/config/aarch64/aarch64.md (aarch64_lshr_sisd_or_int_3):
Place integer variant first.  (aarch64_ashr_sisd_or_int_3):
Likewise.


---
 gcc/config/aarch64/aarch64.md | 27 ++-
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index baef56a..06256e3 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3349,17 +3349,18 @@
 
 ;; Logical right shift using SISD or Integer instruction
 (define_insn "*aarch64_lshr_sisd_or_int_3"
-  [(set (match_operand:GPI 0 "register_operand" "=w,&w,r")
+  [(set (match_operand:GPI 0 "register_operand" "=r,w,&w,&w")
 (lshiftrt:GPI
-  (match_operand:GPI 1 "register_operand" "w,w,r")
-  (match_operand:QI 2 "aarch64_reg_or_shift_imm_" 
"Us,w,rUs")))]
+  (match_operand:GPI 1 "register_operand" "r,w,w,w")
+  (match_operand:QI 2 "aarch64_reg_or_shift_imm_" 
"rUs,Us,w,0")))]
   ""
   "@
+   lsr\t%0, %1, %2
ushr\t%0, %1, %2
#
-   lsr\t%0, %1, %2"
-  [(set_attr "simd" "yes,yes,no")
-   (set_attr "type" "neon_shift_imm,neon_shift_reg,shift_reg")]
+   #"
+  [(set_attr "simd" "no,yes,yes,yes")
+   (set_attr "type" 
"shift_reg,neon_shift_imm,neon_shift_reg,neon_shift_reg")]
 )
 
 (define_split
@@ -3394,18 +3395,18 @@
 
 ;; Arithmetic right shift using SISD or Integer instruction
 (define_insn "*aarch64_ashr_sisd_or_int_3"
-  [(set (match_operand:GPI 0 "register_operand" "=w,&w,&w,r")
+  [(set (match_operand:GPI 0 "register_operand" "=r,w,&w,&w")
 (ashiftrt:GPI
-  (match_operand:GPI 1 "register_operand" "w,w,w,r")
-  (match_operand:QI 2 "aarch64_reg_or_shift_imm_di" 
"Us,w,0,rUs")))]
+  (match_operand:GPI 1 "register_operand" "r,w,w,w")
+  (match_operand:QI 2 "aarch64_reg_or_shift_imm_di" 
"rUs,Us,w,0")))]
   ""
   "@
+   asr\t%0, %1, %2
sshr\t%0, %1, %2
#
-   #
-   asr\t%0, %1, %2"
-  [(set_attr "simd" "yes,yes,yes,no")
-   (set_attr "type" 
"neon_shift_imm,neon_shift_reg,neon_shift_reg,shift_reg")]
+   #"
+  [(set_attr "simd" "no,yes,yes,yes")
+   (set_attr "type" 
"shift_reg,neon_shift_imm,neon_shift_reg,neon_shift_reg")]
 )
 
 (define_split
-- 
1.9.1




Re: [PATCH][AArch64] Improve spill code - swap order in shr patterns

2015-07-27 Thread James Greenhalgh
On Mon, Jul 27, 2015 at 03:38:55PM +0100, Wilco Dijkstra wrote:
> This is basically the same as the shl version but for the shr patterns.
> 
> Various instructions are supported as integer operations as well as SIMD on 
> AArch64. When
> register pressure is high, lra-constraints inserts spill code without taking 
> the allocation
> class into account, and basically chooses the first available pattern that 
> matches. Since this
> instruction has the SIMD version first it is usually chosen eventhough some 
> of the operands
> are eventually allocated to integer registers. The result is inefficient code 
> not only due to
> the higher latency of SIMD instructions but also due to the extra int<->FP 
> moves. Placing the
> integer variant first in the shr pattern generates far more optimal spill 
> code.
> 
> 2015-07-27  Wilco Dijkstra  
> 
> * gcc/config/aarch64/aarch64.md (aarch64_lshr_sisd_or_int_3):
> Place integer variant first.  (aarch64_ashr_sisd_or_int_3):
> Likewise.

This ChangeLog looks odd to me. It should include details of the
new alternative added to aarch64_lshr_sis_or_int_3, and also
be formatted as so:

* gcc/config/aarch64/aarch64.md (aarch64_lshr_sisd_or_int_3):
Place integer variant first.
(aarch64_ashr_sisd_or_int_3): Likewise.

Otherwise, this patch looks OK to me, so I've applied it on your behalf
as revision 226253.

Thanks,
James

> ---
>  gcc/config/aarch64/aarch64.md | 27 ++-
>  1 file changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index baef56a..06256e3 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -3349,17 +3349,18 @@
>  
>  ;; Logical right shift using SISD or Integer instruction
>  (define_insn "*aarch64_lshr_sisd_or_int_3"
> -  [(set (match_operand:GPI 0 "register_operand" "=w,&w,r")
> +  [(set (match_operand:GPI 0 "register_operand" "=r,w,&w,&w")
>  (lshiftrt:GPI
> -  (match_operand:GPI 1 "register_operand" "w,w,r")
> -  (match_operand:QI 2 "aarch64_reg_or_shift_imm_" 
> "Us,w,rUs")))]
> +  (match_operand:GPI 1 "register_operand" "r,w,w,w")
> +  (match_operand:QI 2 "aarch64_reg_or_shift_imm_" 
> "rUs,Us,w,0")))]
>""
>"@
> +   lsr\t%0, %1, %2
> ushr\t%0, %1, %2
> #
> -   lsr\t%0, %1, %2"
> -  [(set_attr "simd" "yes,yes,no")
> -   (set_attr "type" "neon_shift_imm,neon_shift_reg,shift_reg")]
> +   #"
> +  [(set_attr "simd" "no,yes,yes,yes")
> +   (set_attr "type" 
> "shift_reg,neon_shift_imm,neon_shift_reg,neon_shift_reg")]
>  )
>  
>  (define_split
> @@ -3394,18 +3395,18 @@
>  
>  ;; Arithmetic right shift using SISD or Integer instruction
>  (define_insn "*aarch64_ashr_sisd_or_int_3"
> -  [(set (match_operand:GPI 0 "register_operand" "=w,&w,&w,r")
> +  [(set (match_operand:GPI 0 "register_operand" "=r,w,&w,&w")
>  (ashiftrt:GPI
> -  (match_operand:GPI 1 "register_operand" "w,w,w,r")
> -  (match_operand:QI 2 "aarch64_reg_or_shift_imm_di" 
> "Us,w,0,rUs")))]
> +  (match_operand:GPI 1 "register_operand" "r,w,w,w")
> +  (match_operand:QI 2 "aarch64_reg_or_shift_imm_di" 
> "rUs,Us,w,0")))]
>""
>"@
> +   asr\t%0, %1, %2
> sshr\t%0, %1, %2
> #
> -   #
> -   asr\t%0, %1, %2"
> -  [(set_attr "simd" "yes,yes,yes,no")
> -   (set_attr "type" 
> "neon_shift_imm,neon_shift_reg,neon_shift_reg,shift_reg")]
> +   #"
> +  [(set_attr "simd" "no,yes,yes,yes")
> +   (set_attr "type" 
> "shift_reg,neon_shift_imm,neon_shift_reg,neon_shift_reg")]
>  )
>  
>  (define_split
> -- 
> 1.9.1
> 
>