Re: [AArch64] Merge stores of D register values of different modes

[Jackson Woodruff]

On 09/12/2017 07:32 PM, Richard Sandiford wrote:

Thanks for doing this, looks good to me FWIW.  I was just wondering:

Jackson Woodruff  writes:

@@ -14712,6 +14712,11 @@ aarch64_operands_ok_for_ldpstp (rtx *operands, bool 
load,
if (!rtx_equal_p (base_1, base_2))
  return false;
  
+  /* Check that the operands are of the same size.  */

+  if (GET_MODE_SIZE (GET_MODE (mem_1))
+  != GET_MODE_SIZE (GET_MODE (mem_2)))
+return false;
+
offval_1 = INTVAL (offset_1);
offval_2 = INTVAL (offset_2);
msize = GET_MODE_SIZE (mode);


when can this trigger?  Your iterators always seem to enforce correct
pairings, so maybe this should be an assert instead.


Yes, it's true that this should never be triggered. I've changed it to 
an assert.


I have also rebased on top of the renaming of load/store attributes 
patch https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00702.html which had 
some conflicts with this.


Is the updated patch OK for trunk?

Thanks,
Jackson.



Thanks,
Richard

diff --git a/gcc/config/aarch64/aarch64-ldpstp.md b/gcc/config/aarch64/aarch64-ldpstp.md
index e8dda42c2dd1e30c4607c67a2156ff7813bd89ea..14e860d258e548d4118d957675f8bdbb74615337 100644
--- a/gcc/config/aarch64/aarch64-ldpstp.md
+++ b/gcc/config/aarch64/aarch64-ldpstp.md
@@ -99,10 +99,10 @@
 })
 
 (define_peephole2
-  [(set (match_operand:VD 0 "register_operand" "")
-	(match_operand:VD 1 "aarch64_mem_pair_operand" ""))
-   (set (match_operand:VD 2 "register_operand" "")
-	(match_operand:VD 3 "memory_operand" ""))]
+  [(set (match_operand:DREG 0 "register_operand" "")
+	(match_operand:DREG 1 "aarch64_mem_pair_operand" ""))
+   (set (match_operand:DREG2 2 "register_operand" "")
+	(match_operand:DREG2 3 "memory_operand" ""))]
   "aarch64_operands_ok_for_ldpstp (operands, true, mode)"
   [(parallel [(set (match_dup 0) (match_dup 1))
 	  (set (match_dup 2) (match_dup 3))])]
@@ -119,11 +119,12 @@
 })
 
 (define_peephole2
-  [(set (match_operand:VD 0 "aarch64_mem_pair_operand" "")
-	(match_operand:VD 1 "register_operand" ""))
-   (set (match_operand:VD 2 "memory_operand" "")
-	(match_operand:VD 3 "register_operand" ""))]
-  "TARGET_SIMD && aarch64_operands_ok_for_ldpstp (operands, false, mode)"
+  [(set (match_operand:DREG 0 "aarch64_mem_pair_operand" "")
+	(match_operand:DREG 1 "register_operand" ""))
+   (set (match_operand:DREG2 2 "memory_operand" "")
+	(match_operand:DREG2 3 "register_operand" ""))]
+  "TARGET_SIMD
+   && aarch64_operands_ok_for_ldpstp (operands, false, mode)"
   [(parallel [(set (match_dup 0) (match_dup 1))
 	  (set (match_dup 2) (match_dup 3))])]
 {
@@ -138,7 +139,6 @@
 }
 })
 
-
 ;; Handle sign/zero extended consecutive load/store.
 
 (define_peephole2
@@ -181,6 +181,30 @@
 }
 })
 
+;; Handle storing of a floating point zero.
+;; We can match modes that won't work for a stp instruction
+;; as aarch64_operands_ok_for_ldpstp checks that the modes are
+;; compatible.
+(define_peephole2
+  [(set (match_operand:DSX 0 "aarch64_mem_pair_operand" "")
+	(match_operand:DSX 1 "aarch64_reg_zero_or_fp_zero" ""))
+   (set (match_operand: 2 "memory_operand" "")
+	(match_operand: 3 "aarch64_reg_zero_or_fp_zero" ""))]
+  "aarch64_operands_ok_for_ldpstp (operands, false, DImode)"
+  [(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], , _1);
+  extract_base_offset_in_addr (operands[2], , _2);
+  if (INTVAL (offset_1) > INTVAL (offset_2))
+{
+  std::swap (operands[0], operands[2]);
+  std::swap (operands[1], operands[3]);
+}
+})
+
 ;; Handle consecutive load/store whose offset is out of the range
 ;; supported by ldp/ldpsw/stp.  We firstly adjust offset in a scratch
 ;; register, then merge them into ldp/ldpsw/stp by using the adjusted
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 8f045c210502330af9d47f6adfd46a9e36328b74..90f9415b3986eb737ecdfeed43fe798cdbb8334e 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -172,11 +172,11 @@
   [(set_attr "type" "neon_store1_1reg")]
 )
 
-(define_insn "load_pair"
-  [(set (match_operand:VD 0 "register_operand" "=w")
-	(match_operand:VD 1 "aarch64_mem_pair_operand" "Ump"))
-   (set (match_operand:VD 2 "register_operand" "=w")
-	(match_operand:VD 3 "memory_operand" "m"))]
+(define_insn "load_pair"
+  [(set (match_operand:DREG 0 "register_operand" "=w")
+	(match_operand:DREG 1 "aarch64_mem_pair_operand" "Ump"))
+   (set (match_operand:DREG2 2 "register_operand" "=w")
+	(match_operand:DREG2 3 "memory_operand" "m"))]
   "TARGET_SIMD
&& rtx_equal_p (XEXP (operands[3], 0),
 		   plus_constant (Pmode,
@@ -186,11 +186,11 @@
   [(set_attr "type" "neon_ldp")]
 )
 
-(define_insn "store_pair"
-  [(set (match_operand:VD 0 "aarch64_mem_pair_operand" "=Ump")
-	(match_operand:VD 1 "register_operand" "w"))
-   (set 

Re: [AArch64] Merge stores of D register values of different modes

[Richard Sandiford]

Thanks for doing this, looks good to me FWIW.  I was just wondering:

Jackson Woodruff  writes:
> @@ -14712,6 +14712,11 @@ aarch64_operands_ok_for_ldpstp (rtx *operands, bool 
> load,
>if (!rtx_equal_p (base_1, base_2))
>  return false;
>  
> +  /* Check that the operands are of the same size.  */
> +  if (GET_MODE_SIZE (GET_MODE (mem_1))
> +  != GET_MODE_SIZE (GET_MODE (mem_2)))
> +return false;
> +
>offval_1 = INTVAL (offset_1);
>offval_2 = INTVAL (offset_2);
>msize = GET_MODE_SIZE (mode);

when can this trigger?  Your iterators always seem to enforce correct
pairings, so maybe this should be an assert instead.

Thanks,
Richard