[AArch64] Refactor of aarch64-ldpstp

Message ID 5B0431E7.8020705@foss.arm.com
State New
Headers show
Series
  • [AArch64] Refactor of aarch64-ldpstp
Related show

Commit Message

Kyrill Tkachov May 22, 2018, 3:06 p.m.
[sending on behalf of Jackson Woodruff]

Hi all,

This patch removes a lot of duplicated code in aarch64-ldpstp.md.

The patterns that did not previously generate a base register now
do not check for aarch64_mem_pair_operand in the pattern. This has
been extracted to a check in aarch64_operands_ok_for_ldpstp.

All patterns in the file used to have explicit switching code to
swap loads and stores that were in the wrong order.

This has been extracted into aarch64_operands_ok_for_ldpstp
as a final operation after all the checks have been performed.

This patch is based on the patch here: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01129.html


Bootstrap and regtest OK on AArch64.

OK for trunk?

Jackson.

gcc/

2018-05-22  Jackson Woodruff  <jackson.woodruff@arm.com>
             Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/aarch64/aarch64-ldpstp.md: Replace uses of
     aarch64_mem_pair_operand with memory_operand and delete operand swapping
     code.
     * config/aarch64/aarch64.c (aarch64_operands_ok_for_ldpstp):
     Add check for legitimate_address.
     (aarch64_gen_adjusted_ldpstp): Swap operands where appropriate.
     (aarch64_swap_ldrstr_operands): New.
     * config/aarch64/aarch64-protos.h (aarch64_swap_ldrstr_operands):
     Define prototype.

Comments

James Greenhalgh May 22, 2018, 3:30 p.m. | #1
On Tue, May 22, 2018 at 10:06:15AM -0500, Kyrill Tkachov wrote:
> [sending on behalf of Jackson Woodruff]

> 

> Hi all,

> 

> This patch removes a lot of duplicated code in aarch64-ldpstp.md.

> 

> The patterns that did not previously generate a base register now

> do not check for aarch64_mem_pair_operand in the pattern. This has

> been extracted to a check in aarch64_operands_ok_for_ldpstp.

> 

> All patterns in the file used to have explicit switching code to

> swap loads and stores that were in the wrong order.

> 

> This has been extracted into aarch64_operands_ok_for_ldpstp

> as a final operation after all the checks have been performed.

> 

> This patch is based on the patch here: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01129.html

> 

> 

> Bootstrap and regtest OK on AArch64.

> 

> OK for trunk?


OK.

Thnaks,
James

> 

> Jackson.

> 

> gcc/

> 

> 2018-05-22  Jackson Woodruff  <jackson.woodruff@arm.com>

>              Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

> 

>      * config/aarch64/aarch64-ldpstp.md: Replace uses of

>      aarch64_mem_pair_operand with memory_operand and delete operand swapping

>      code.

>      * config/aarch64/aarch64.c (aarch64_operands_ok_for_ldpstp):

>      Add check for legitimate_address.

>      (aarch64_gen_adjusted_ldpstp): Swap operands where appropriate.

>      (aarch64_swap_ldrstr_operands): New.

>      * config/aarch64/aarch64-protos.h (aarch64_swap_ldrstr_operands):

>      Define prototype.

Patch

diff --git a/gcc/config/aarch64/aarch64-ldpstp.md b/gcc/config/aarch64/aarch64-ldpstp.md
index f6fe8a6a93b5466723e3ed6b892e0ec1e67ee89d..7f1031dc80fab31f691c0b03d6a485c1b6fd7e53 100644
--- a/gcc/config/aarch64/aarch64-ldpstp.md
+++ b/gcc/config/aarch64/aarch64-ldpstp.md
@@ -20,26 +20,18 @@ 
 
 (define_peephole2
   [(set (match_operand:GPI 0 "register_operand" "")
-	(match_operand:GPI 1 "aarch64_mem_pair_operand" ""))
+	(match_operand:GPI 1 "memory_operand" ""))
    (set (match_operand:GPI 2 "register_operand" "")
 	(match_operand:GPI 3 "memory_operand" ""))]
   "aarch64_operands_ok_for_ldpstp (operands, true, <MODE>mode)"
   [(parallel [(set (match_dup 0) (match_dup 1))
 	      (set (match_dup 2) (match_dup 3))])]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[1], &base, &offset_1);
-  extract_base_offset_in_addr (operands[3], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[2]);
-      std::swap (operands[1], operands[3]);
-    }
+  aarch64_swap_ldrstr_operands (operands, true);
 })
 
 (define_peephole2
-  [(set (match_operand:GPI 0 "aarch64_mem_pair_operand" "")
+  [(set (match_operand:GPI 0 "memory_operand" "")
 	(match_operand:GPI 1 "aarch64_reg_or_zero" ""))
    (set (match_operand:GPI 2 "memory_operand" "")
 	(match_operand:GPI 3 "aarch64_reg_or_zero" ""))]
@@ -47,39 +39,23 @@  (define_peephole2
   [(parallel [(set (match_dup 0) (match_dup 1))
 	      (set (match_dup 2) (match_dup 3))])]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[0], &base, &offset_1);
-  extract_base_offset_in_addr (operands[2], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[2]);
-      std::swap (operands[1], operands[3]);
-    }
+  aarch64_swap_ldrstr_operands (operands, false);
 })
 
 (define_peephole2
   [(set (match_operand:GPF 0 "register_operand" "")
-	(match_operand:GPF 1 "aarch64_mem_pair_operand" ""))
+	(match_operand:GPF 1 "memory_operand" ""))
    (set (match_operand:GPF 2 "register_operand" "")
 	(match_operand:GPF 3 "memory_operand" ""))]
   "aarch64_operands_ok_for_ldpstp (operands, true, <MODE>mode)"
   [(parallel [(set (match_dup 0) (match_dup 1))
 	      (set (match_dup 2) (match_dup 3))])]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[1], &base, &offset_1);
-  extract_base_offset_in_addr (operands[3], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[2]);
-      std::swap (operands[1], operands[3]);
-    }
+  aarch64_swap_ldrstr_operands (operands, true);
 })
 
 (define_peephole2
-  [(set (match_operand:GPF 0 "aarch64_mem_pair_operand" "")
+  [(set (match_operand:GPF 0 "memory_operand" "")
 	(match_operand:GPF 1 "aarch64_reg_or_fp_zero" ""))
    (set (match_operand:GPF 2 "memory_operand" "")
 	(match_operand:GPF 3 "aarch64_reg_or_fp_zero" ""))]
@@ -87,39 +63,23 @@  (define_peephole2
   [(parallel [(set (match_dup 0) (match_dup 1))
 	      (set (match_dup 2) (match_dup 3))])]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[0], &base, &offset_1);
-  extract_base_offset_in_addr (operands[2], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[2]);
-      std::swap (operands[1], operands[3]);
-    }
+  aarch64_swap_ldrstr_operands (operands, false);
 })
 
 (define_peephole2
   [(set (match_operand:DREG 0 "register_operand" "")
-	(match_operand:DREG 1 "aarch64_mem_pair_operand" ""))
+	(match_operand:DREG 1 "memory_operand" ""))
    (set (match_operand:DREG2 2 "register_operand" "")
 	(match_operand:DREG2 3 "memory_operand" ""))]
   "aarch64_operands_ok_for_ldpstp (operands, true, <DREG:MODE>mode)"
   [(parallel [(set (match_dup 0) (match_dup 1))
 	      (set (match_dup 2) (match_dup 3))])]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[1], &base, &offset_1);
-  extract_base_offset_in_addr (operands[3], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[2]);
-      std::swap (operands[1], operands[3]);
-    }
+  aarch64_swap_ldrstr_operands (operands, true);
 })
 
 (define_peephole2
-  [(set (match_operand:DREG 0 "aarch64_mem_pair_operand" "")
+  [(set (match_operand:DREG 0 "memory_operand" "")
 	(match_operand:DREG 1 "register_operand" ""))
    (set (match_operand:DREG2 2 "memory_operand" "")
 	(match_operand:DREG2 3 "register_operand" ""))]
@@ -128,57 +88,33 @@  (define_peephole2
   [(parallel [(set (match_dup 0) (match_dup 1))
 	      (set (match_dup 2) (match_dup 3))])]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[0], &base, &offset_1);
-  extract_base_offset_in_addr (operands[2], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[2]);
-      std::swap (operands[1], operands[3]);
-    }
+  aarch64_swap_ldrstr_operands (operands, false);
 })
 
 ;; Handle sign/zero extended consecutive load/store.
 
 (define_peephole2
   [(set (match_operand:DI 0 "register_operand" "")
-	(sign_extend:DI (match_operand:SI 1 "aarch64_mem_pair_operand" "")))
+	(sign_extend:DI (match_operand:SI 1 "memory_operand" "")))
    (set (match_operand:DI 2 "register_operand" "")
 	(sign_extend:DI (match_operand:SI 3 "memory_operand" "")))]
   "aarch64_operands_ok_for_ldpstp (operands, true, SImode)"
   [(parallel [(set (match_dup 0) (sign_extend:DI (match_dup 1)))
 	      (set (match_dup 2) (sign_extend:DI (match_dup 3)))])]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[1], &base, &offset_1);
-  extract_base_offset_in_addr (operands[3], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[2]);
-      std::swap (operands[1], operands[3]);
-    }
+  aarch64_swap_ldrstr_operands (operands, true);
 })
 
 (define_peephole2
   [(set (match_operand:DI 0 "register_operand" "")
-	(zero_extend:DI (match_operand:SI 1 "aarch64_mem_pair_operand" "")))
+	(zero_extend:DI (match_operand:SI 1 "memory_operand" "")))
    (set (match_operand:DI 2 "register_operand" "")
 	(zero_extend:DI (match_operand:SI 3 "memory_operand" "")))]
   "aarch64_operands_ok_for_ldpstp (operands, true, SImode)"
   [(parallel [(set (match_dup 0) (zero_extend:DI (match_dup 1)))
 	      (set (match_dup 2) (zero_extend:DI (match_dup 3)))])]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[1], &base, &offset_1);
-  extract_base_offset_in_addr (operands[3], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[2]);
-      std::swap (operands[1], operands[3]);
-    }
+  aarch64_swap_ldrstr_operands (operands, true);
 })
 
 ;; Handle storing of a floating point zero with integer data.
@@ -192,7 +128,7 @@  (define_peephole2
 ;; as aarch64_operands_ok_for_ldpstp checks that the modes are
 ;; compatible.
 (define_peephole2
-  [(set (match_operand:DSX 0 "aarch64_mem_pair_operand" "")
+  [(set (match_operand:DSX 0 "memory_operand" "")
 	(match_operand:DSX 1 "aarch64_reg_zero_or_fp_zero" ""))
    (set (match_operand:<FCVT_TARGET> 2 "memory_operand" "")
 	(match_operand:<FCVT_TARGET> 3 "aarch64_reg_zero_or_fp_zero" ""))]
@@ -200,15 +136,7 @@  (define_peephole2
   [(parallel [(set (match_dup 0) (match_dup 1))
 	      (set (match_dup 2) (match_dup 3))])]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[0], &base, &offset_1);
-  extract_base_offset_in_addr (operands[2], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[2]);
-      std::swap (operands[1], operands[3]);
-    }
+  aarch64_swap_ldrstr_operands (operands, false);
 })
 
 ;; Handle consecutive load/store whose offset is out of the range
@@ -230,18 +158,6 @@  (define_peephole2
   "aarch64_operands_adjust_ok_for_ldpstp (operands, true, <MODE>mode)"
   [(const_int 0)]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[1], &base, &offset_1);
-  extract_base_offset_in_addr (operands[3], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[6]);
-      std::swap (operands[1], operands[7]);
-      std::swap (operands[2], operands[4]);
-      std::swap (operands[3], operands[5]);
-    }
-
   if (aarch64_gen_adjusted_ldpstp (operands, true, <MODE>mode, UNKNOWN))
     DONE;
   else
@@ -262,18 +178,6 @@  (define_peephole2
   "aarch64_operands_adjust_ok_for_ldpstp (operands, true, <MODE>mode)"
   [(const_int 0)]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[1], &base, &offset_1);
-  extract_base_offset_in_addr (operands[3], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[6]);
-      std::swap (operands[1], operands[7]);
-      std::swap (operands[2], operands[4]);
-      std::swap (operands[3], operands[5]);
-    }
-
   if (aarch64_gen_adjusted_ldpstp (operands, true, <MODE>mode, UNKNOWN))
     DONE;
   else
@@ -294,18 +198,6 @@  (define_peephole2
   "aarch64_operands_adjust_ok_for_ldpstp (operands, true, SImode)"
   [(const_int 0)]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[1], &base, &offset_1);
-  extract_base_offset_in_addr (operands[3], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[6]);
-      std::swap (operands[1], operands[7]);
-      std::swap (operands[2], operands[4]);
-      std::swap (operands[3], operands[5]);
-    }
-
   if (aarch64_gen_adjusted_ldpstp (operands, true, SImode, SIGN_EXTEND))
     DONE;
   else
@@ -326,18 +218,6 @@  (define_peephole2
   "aarch64_operands_adjust_ok_for_ldpstp (operands, true, SImode)"
   [(const_int 0)]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[1], &base, &offset_1);
-  extract_base_offset_in_addr (operands[3], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[6]);
-      std::swap (operands[1], operands[7]);
-      std::swap (operands[2], operands[4]);
-      std::swap (operands[3], operands[5]);
-    }
-
   if (aarch64_gen_adjusted_ldpstp (operands, true, SImode, ZERO_EXTEND))
     DONE;
   else
@@ -358,18 +238,6 @@  (define_peephole2
   "aarch64_operands_adjust_ok_for_ldpstp (operands, false, <MODE>mode)"
   [(const_int 0)]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[0], &base, &offset_1);
-  extract_base_offset_in_addr (operands[2], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[6]);
-      std::swap (operands[1], operands[7]);
-      std::swap (operands[2], operands[4]);
-      std::swap (operands[3], operands[5]);
-    }
-
   if (aarch64_gen_adjusted_ldpstp (operands, false, <MODE>mode, UNKNOWN))
     DONE;
   else
@@ -390,18 +258,6 @@  (define_peephole2
   "aarch64_operands_adjust_ok_for_ldpstp (operands, false, <MODE>mode)"
   [(const_int 0)]
 {
-  rtx base, offset_1, offset_2;
-
-  extract_base_offset_in_addr (operands[0], &base, &offset_1);
-  extract_base_offset_in_addr (operands[2], &base, &offset_2);
-  if (INTVAL (offset_1) > INTVAL (offset_2))
-    {
-      std::swap (operands[0], operands[6]);
-      std::swap (operands[1], operands[7]);
-      std::swap (operands[2], operands[4]);
-      std::swap (operands[3], operands[5]);
-    }
-
   if (aarch64_gen_adjusted_ldpstp (operands, false, <MODE>mode, UNKNOWN))
     DONE;
   else
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index cda2895d28e7496f8fd6c1b365c4bb497b54c323..4ab68110eb4602939582fa4498cca686468bebb6 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -534,6 +534,7 @@  int aarch64_ccmp_mode_to_code (machine_mode mode);
 bool extract_base_offset_in_addr (rtx mem, rtx *base, rtx *offset);
 bool aarch64_operands_ok_for_ldpstp (rtx *, bool, machine_mode);
 bool aarch64_operands_adjust_ok_for_ldpstp (rtx *, bool, scalar_mode);
+void aarch64_swap_ldrstr_operands (rtx *, bool);
 
 extern void aarch64_asm_output_pool_epilogue (FILE *, const char *,
 					      tree, HOST_WIDE_INT);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index b75a588eb9aa49b1796161d34494d452a6742e4d..f60e0ad37565b044b3ffe446eebcc0f40f9d99db 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -16874,9 +16874,16 @@  aarch64_operands_ok_for_ldpstp (rtx *operands, bool load,
 
       /* In increasing order, the last load can clobber the address.  */
       if (offval_1 > offval_2 && reg_mentioned_p (reg_2, mem_2))
-      return false;
+	return false;
     }
 
+  /* One of the memory accesses must be a mempair operand.
+     If it is not the first one, they need to be swapped by the
+     peephole.  */
+  if (!aarch64_mem_pair_operand (mem_1, GET_MODE (mem_1))
+       && !aarch64_mem_pair_operand (mem_2, GET_MODE (mem_2)))
+    return false;
+
   if (REG_P (reg_1) && FP_REGNUM_P (REGNO (reg_1)))
     rclass_1 = FP_REGS;
   else
@@ -16894,6 +16901,40 @@  aarch64_operands_ok_for_ldpstp (rtx *operands, bool load,
   return true;
 }
 
+/* Given OPERANDS of consecutive load/store that can be merged,
+   swap them if they are not in ascending order.  */
+void
+aarch64_swap_ldrstr_operands (rtx* operands, bool load)
+{
+  rtx mem_1, mem_2, base_1, base_2, offset_1, offset_2;
+  HOST_WIDE_INT offval_1, offval_2;
+
+  if (load)
+    {
+      mem_1 = operands[1];
+      mem_2 = operands[3];
+    }
+  else
+    {
+      mem_1 = operands[0];
+      mem_2 = operands[2];
+    }
+
+  extract_base_offset_in_addr (mem_1, &base_1, &offset_1);
+  extract_base_offset_in_addr (mem_2, &base_2, &offset_2);
+
+  offval_1 = INTVAL (offset_1);
+  offval_2 = INTVAL (offset_2);
+
+  if (offval_1 > offval_2)
+    {
+      /* Irrespective of whether this is a load or a store,
+	 we do the same swap.  */
+      std::swap (operands[0], operands[2]);
+      std::swap (operands[1], operands[3]);
+    }
+}
+
 /* Given OPERANDS of consecutive load/store, check if we can merge
    them into ldp/stp by adjusting the offset.  LOAD is true if they
    are load instructions.  MODE is the mode of memory operands.
@@ -17053,7 +17094,7 @@  bool
 aarch64_gen_adjusted_ldpstp (rtx *operands, bool load,
 			     scalar_mode mode, RTX_CODE code)
 {
-  rtx base, offset, t1, t2;
+  rtx base, offset_1, offset_2, t1, t2;
   rtx mem_1, mem_2, mem_3, mem_4;
   HOST_WIDE_INT off_val, abs_off, adj_off, new_off, stp_off_limit, msize;
 
@@ -17061,6 +17102,30 @@  aarch64_gen_adjusted_ldpstp (rtx *operands, bool load,
     {
       mem_1 = operands[1];
       mem_2 = operands[3];
+    }
+  else
+    {
+      mem_1 = operands[0];
+      mem_2 = operands[2];
+    }
+
+  extract_base_offset_in_addr (mem_1, &base, &offset_1);
+  extract_base_offset_in_addr (mem_2, &base, &offset_2);
+  gcc_assert (base != NULL_RTX && offset_1 != NULL_RTX
+	      && offset_2 != NULL_RTX);
+
+  if (INTVAL (offset_1) > INTVAL (offset_2))
+    {
+      std::swap (operands[0], operands[6]);
+      std::swap (operands[1], operands[7]);
+      std::swap (operands[2], operands[4]);
+      std::swap (operands[3], operands[5]);
+    }
+
+  if (load)
+    {
+      mem_1 = operands[1];
+      mem_2 = operands[3];
       mem_3 = operands[5];
       mem_4 = operands[7];
     }
@@ -17073,13 +17138,14 @@  aarch64_gen_adjusted_ldpstp (rtx *operands, bool load,
       gcc_assert (code == UNKNOWN);
     }
 
-  extract_base_offset_in_addr (mem_1, &base, &offset);
-  gcc_assert (base != NULL_RTX && offset != NULL_RTX);
+  /* Extract the offset of the new first address.  */
+  extract_base_offset_in_addr (mem_1, &base, &offset_1);
+  extract_base_offset_in_addr (mem_2, &base, &offset_2);
 
   /* Adjust offset thus it can fit in ldp/stp instruction.  */
   msize = GET_MODE_SIZE (mode);
   stp_off_limit = msize * 0x40;
-  off_val = INTVAL (offset);
+  off_val = INTVAL (offset_1);
   abs_off = (off_val < 0) ? -off_val : off_val;
   new_off = abs_off % stp_off_limit;
   adj_off = abs_off - new_off;