public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Hurugalawadi, Naveen" <Naveen.Hurugalawadi@cavium.com>
To: James Greenhalgh <james.greenhalgh@arm.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
	"Pinski, Andrew"	<Andrew.Pinski@cavium.com>,
	Marcus Shawcroft <marcus.shawcroft@arm.com>,
	Richard Earnshaw <Richard.Earnshaw@arm.com>,
	"nd@arm.com" <nd@arm.com>
Subject: Re: [PATCH][AArch64] Add addr_type attribute
Date: Wed, 26 Jul 2017 07:56:00 -0000	[thread overview]
Message-ID: <CO2PR07MB26937220B9C79CDDC9E6988683B90@CO2PR07MB2693.namprd07.prod.outlook.com> (raw)
In-Reply-To: <20170614143916.GE8010@arm.com>

[-- Attachment #1: Type: text/plain, Size: 702 bytes --]

Hi James,

Thanks for the review and comments on the patch.

>> What am I missing - you add a new function which is never called?
>> Should this have been in series with a scheduling model change?

Sorry. You are right. This patch is one in series for scheduling and
addition of attributes to improve the performance. 
The function is part of the other patch which will be posted after testing.

>> Note you need to include the POST ones for AARCH64 but
>> it should be similar enough.

Modified the patch as per your suggestion as in PowerPC.

Please review the patch and let me know your comments on it.
Bootstrapped and Regression tested on aarch64-thunder-linux.

Thanks,
Naveen

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: addr-type-1.patch --]
[-- Type: text/x-diff; name="addr-type-1.patch", Size: 16636 bytes --]

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index f876a2b..0fb62fc 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -212,6 +212,30 @@
 ;; no predicated insns.
 (define_attr "predicated" "yes,no" (const_string "no"))
 
+;; Does this instruction use indexed (that is, reg+reg) addressing?
+;; This is used for load and store insns.  If operand 0 or 1 is a MEM
+;; it is automatically set based on that.  If a load or store instruction
+;; has fewer than two operands it needs to set this attribute manually
+;; or the compiler will crash.
+(define_attr "index" "no,yes"
+  (if_then_else (ior (match_operand 0 "index_address_mem")
+                     (match_operand 1 "index_address_mem"))
+                (const_string "yes")
+                (const_string "no")))
+
+;; Does this instruction use update addressing?
+;; This is used for load and store insns.  See the comments for "indexed".
+(define_attr "update" "no,yes"
+  (if_then_else (ior (match_operand 0 "update_address_mem")
+                     (match_operand 1 "update_address_mem"))
+                (const_string "yes")
+                (const_string "no")))
+
+(define_attr "index_shift" "no,yes"
+  (if_then_else (ior (match_operand 0 "index_shift_address_mem")
+                     (match_operand 1 "index_shift_address_mem"))
+                (const_string "yes")
+                (const_string "no")))
 ;; -------------------------------------------------------------------
 ;; Pipeline descriptions and scheduling
 ;; -------------------------------------------------------------------
@@ -546,7 +570,19 @@
     operands[0] = gen_rtx_MEM (DImode, operands[0]);
     return pftype[INTVAL(operands[1])][locality];
   }
-  [(set_attr "type" "load1")]
+  [(set_attr "type" "load1")
+   (set (attr "update")
+	(if_then_else (match_operand 0 "update_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 0 "index_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 0 "index_shift_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))]
 )
 
 (define_insn "trap"
@@ -1192,7 +1228,19 @@
    ldp\\t%w0, %w2, %1
    ldp\\t%s0, %s2, %1"
   [(set_attr "type" "load2,neon_load1_2reg")
-   (set_attr "fp" "*,yes")]
+   (set_attr "fp" "*,yes")
+   (set (attr "update")
+	(if_then_else (match_operand 1 "update_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 1 "index_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 1 "index_shift_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))]
 )
 
 (define_insn "load_pairdi"
@@ -1208,7 +1256,19 @@
    ldp\\t%x0, %x2, %1
    ldp\\t%d0, %d2, %1"
   [(set_attr "type" "load2,neon_load1_2reg")
-   (set_attr "fp" "*,yes")]
+   (set_attr "fp" "*,yes")
+   (set (attr "update")
+	(if_then_else (match_operand 1 "update_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 1 "index_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 1 "index_shift_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))]
 )
 
 
@@ -1227,7 +1287,19 @@
    stp\\t%w1, %w3, %0
    stp\\t%s1, %s3, %0"
   [(set_attr "type" "store2,neon_store1_2reg")
-   (set_attr "fp" "*,yes")]
+   (set_attr "fp" "*,yes")
+   (set (attr "update")
+	(if_then_else (match_operand 0 "update_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 0 "index_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 0 "index_shift_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))]
 )
 
 (define_insn "store_pairdi"
@@ -1243,7 +1315,19 @@
    stp\\t%x1, %x3, %0
    stp\\t%d1, %d3, %0"
   [(set_attr "type" "store2,neon_store1_2reg")
-   (set_attr "fp" "*,yes")]
+   (set_attr "fp" "*,yes")
+   (set (attr "update")
+	(if_then_else (match_operand 0 "update_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 0 "index_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 0 "index_shift_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))]
 )
 
 ;; Operands 1 and 3 are tied together by the final condition; so we allow
@@ -1261,7 +1345,19 @@
    ldp\\t%s0, %s2, %1
    ldp\\t%w0, %w2, %1"
   [(set_attr "type" "neon_load1_2reg,load2")
-   (set_attr "fp" "yes,*")]
+   (set_attr "fp" "yes,*")
+   (set (attr "update")
+	(if_then_else (match_operand 1 "update_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 1 "index_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 1 "index_shift_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))]
 )
 
 (define_insn "load_pairdf"
@@ -1277,7 +1373,19 @@
    ldp\\t%d0, %d2, %1
    ldp\\t%x0, %x2, %1"
   [(set_attr "type" "neon_load1_2reg,load2")
-   (set_attr "fp" "yes,*")]
+   (set_attr "fp" "yes,*")
+   (set (attr "update")
+	(if_then_else (match_operand 1 "update_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 1 "index_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 1 "index_shift_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))]
 )
 
 ;; Operands 0 and 2 are tied together by the final condition; so we allow
@@ -1295,7 +1403,19 @@
    stp\\t%s1, %s3, %0
    stp\\t%w1, %w3, %0"
   [(set_attr "type" "neon_store1_2reg,store2")
-   (set_attr "fp" "yes,*")]
+   (set_attr "fp" "yes,*")
+   (set (attr "update")
+	(if_then_else (match_operand 0 "update_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 0 "index_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 0 "index_shift_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))]
 )
 
 (define_insn "store_pairdf"
@@ -1311,7 +1431,19 @@
    stp\\t%d1, %d3, %0
    stp\\t%x1, %x3, %0"
   [(set_attr "type" "neon_store1_2reg,store2")
-   (set_attr "fp" "yes,*")]
+   (set_attr "fp" "yes,*")
+   (set (attr "update")
+	(if_then_else (match_operand 0 "update_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 0 "index_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 0 "index_shift_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))]
 )
 
 ;; Load pair with post-index writeback.  This is primarily used in function
@@ -1328,7 +1460,8 @@
                    (match_operand:P 5 "const_int_operand" "n"))))])]
   "INTVAL (operands[5]) == GET_MODE_SIZE (<GPI:MODE>mode)"
   "ldp\\t%<w>2, %<w>3, [%1], %4"
-  [(set_attr "type" "load2")]
+  [(set_attr "type" "load2")
+   (set_attr "update" "yes")]
 )
 
 (define_insn "loadwb_pair<GPF:mode>_<P:mode>"
@@ -1361,7 +1494,8 @@
           (match_operand:GPI 3 "register_operand" "r"))])]
   "INTVAL (operands[5]) == INTVAL (operands[4]) + GET_MODE_SIZE (<GPI:MODE>mode)"
   "stp\\t%<w>2, %<w>3, [%0, %4]!"
-  [(set_attr "type" "store2")]
+  [(set_attr "type" "store2")
+   (set_attr "update" "yes")]
 )
 
 (define_insn "storewb_pair<GPF:mode>_<P:mode>"
@@ -1397,7 +1531,28 @@
   "@
    sxtw\t%0, %w1
    ldrsw\t%0, %1"
-  [(set_attr "type" "extend,load1")]
+  [(set_attr "type" "extend,load1")
+   (set (attr "update")
+     (cond [(eq_attr "alternative" "1")
+	      (if_then_else (match_operand 1 "update_address_mem")
+			    (const_string "yes")
+			    (const_string "no"))
+	   ]
+	   (const_string "*")))
+   (set (attr "index")
+     (cond [(eq_attr "alternative" "1")
+	      (if_then_else (match_operand 1 "index_address_mem")
+			    (const_string "yes")
+			    (const_string "no"))
+	   ]
+	   (const_string "*")))
+   (set (attr "index_shift")
+     (cond [(eq_attr "alternative" "1")
+	      (if_then_else (match_operand 1 "index_shift_address_mem")
+			    (const_string "yes")
+			    (const_string "no"))
+	   ]
+	   (const_string "*")))]
 )
 
 (define_insn "*load_pair_extendsidi2_aarch64"
@@ -1410,7 +1565,19 @@
 			       XEXP (operands[1], 0),
 			       GET_MODE_SIZE (SImode)))"
   "ldpsw\\t%0, %2, %1"
-  [(set_attr "type" "load2")]
+  [(set_attr "type" "load2")
+   (set (attr "update")
+	(if_then_else (match_operand 1 "update_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 1 "index_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 1 "index_shift_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))]
 )
 
 (define_insn "*zero_extendsidi2_aarch64"
@@ -1420,7 +1587,28 @@
   "@
    uxtw\t%0, %w1
    ldr\t%w0, %1"
-  [(set_attr "type" "extend,load1")]
+  [(set_attr "type" "extend,load1")
+   (set (attr "update")
+     (cond [(eq_attr "alternative" "1")
+	      (if_then_else (match_operand 1 "update_address_mem")
+			    (const_string "yes")
+			    (const_string "no"))
+	   ]
+	   (const_string "*")))
+   (set (attr "index")
+     (cond [(eq_attr "alternative" "1")
+	      (if_then_else (match_operand 1 "index_address_mem")
+			    (const_string "yes")
+			    (const_string "no"))
+	   ]
+	   (const_string "*")))
+   (set (attr "index_shift")
+     (cond [(eq_attr "alternative" "1")
+	      (if_then_else (match_operand 1 "index_shift_address_mem")
+			    (const_string "yes")
+			    (const_string "no"))
+	   ]
+	   (const_string "*")))]
 )
 
 (define_insn "*load_pair_zero_extendsidi2_aarch64"
@@ -1433,7 +1621,19 @@
 			       XEXP (operands[1], 0),
 			       GET_MODE_SIZE (SImode)))"
   "ldp\\t%w0, %w2, %1"
-  [(set_attr "type" "load2")]
+  [(set_attr "type" "load2")
+   (set (attr "update")
+	(if_then_else (match_operand 1 "update_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 1 "index_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 1 "index_shift_address_mem")
+		      (const_string "yes")
+		      (const_string "no")))]
 )
 
 (define_expand "<ANY_EXTEND:optab><SHORT:mode><GPI:mode>2"
@@ -1449,7 +1649,28 @@
   "@
    sxt<SHORT:size>\t%<GPI:w>0, %w1
    ldrs<SHORT:size>\t%<GPI:w>0, %1"
-  [(set_attr "type" "extend,load1")]
+  [(set_attr "type" "extend,load1")
+   (set (attr "update")
+     (cond [(eq_attr "alternative" "1")
+	      (if_then_else (match_operand 1 "update_address_mem")
+			    (const_string "yes")
+			    (const_string "no"))
+	   ]
+	   (const_string "*")))
+   (set (attr "index")
+     (cond [(eq_attr "alternative" "1")
+	      (if_then_else (match_operand 1 "index_address_mem")
+			    (const_string "yes")
+			    (const_string "no"))
+	   ]
+	   (const_string "*")))
+   (set (attr "index_shift")
+     (cond [(eq_attr "alternative" "1")
+	      (if_then_else (match_operand 1 "index_shift_address_mem")
+			    (const_string "yes")
+			    (const_string "no"))
+	   ]
+	   (const_string "*")))]
 )
 
 (define_insn "*zero_extend<SHORT:mode><GPI:mode>2_aarch64"
@@ -1460,7 +1681,28 @@
    and\t%<GPI:w>0, %<GPI:w>1, <SHORT:short_mask>
    ldr<SHORT:size>\t%w0, %1
    ldr\t%<SHORT:size>0, %1"
-  [(set_attr "type" "logic_imm,load1,load1")]
+  [(set_attr "type" "logic_imm,load1,load1")
+   (set (attr "update")
+     (cond [(eq_attr "alternative" "1,2")
+	      (if_then_else (match_operand 1 "update_address_mem")
+			    (const_string "yes")
+			    (const_string "no"))
+	   ]
+	   (const_string "*")))
+   (set (attr "index")
+     (cond [(eq_attr "alternative" "1,2")
+	      (if_then_else (match_operand 1 "index_address_mem")
+			    (const_string "yes")
+			    (const_string "no"))
+	   ]
+	   (const_string "*")))
+   (set (attr "index_shift")
+     (cond [(eq_attr "alternative" "1,2")
+	      (if_then_else (match_operand 1 "index_shift_address_mem")
+			    (const_string "yes")
+			    (const_string "no"))
+	   ]
+	   (const_string "*")))]
 )
 
 (define_expand "<optab>qihi2"
@@ -5239,7 +5481,10 @@
 		    UNSPEC_GOTSMALLPIC))]
   ""
   "ldr\\t%<w>0, [%1, #:got_lo12:%a2]"
-  [(set_attr "type" "load1")]
+  [(set_attr "type" "load1")
+   (set_attr "update" "no")
+   (set_attr "index" "no")
+   (set_attr "index_shift" "no")]
 )
 
 (define_insn "ldr_got_small_sidi"
@@ -5251,7 +5496,10 @@
 		    UNSPEC_GOTSMALLPIC)))]
   "TARGET_ILP32"
   "ldr\\t%w0, [%1, #:got_lo12:%a2]"
-  [(set_attr "type" "load1")]
+  [(set_attr "type" "load1")
+   (set_attr "update" "no")
+   (set_attr "index" "no")
+   (set_attr "index_shift" "no")]
 )
 
 (define_insn "ldr_got_small_28k_<mode>"
@@ -5283,7 +5531,10 @@
 		   UNSPEC_GOTTINYPIC))]
   ""
   "ldr\\t%0, %L1"
-  [(set_attr "type" "load1")]
+  [(set_attr "type" "load1")
+   (set_attr "update" "no")
+   (set_attr "index" "no")
+   (set_attr "index_shift" "no")]
 )
 
 (define_insn "aarch64_load_tp_hard"
@@ -5325,7 +5576,10 @@
   ""
   "adrp\\t%0, %A1\;ldr\\t%<w>0, [%0, #%L1]"
   [(set_attr "type" "load1")
-   (set_attr "length" "8")]
+   (set_attr "length" "8")
+   (set_attr "update" "no")
+   (set_attr "index" "no")
+   (set_attr "index_shift" "no")]
 )
 
 (define_insn "tlsie_small_sidi"
@@ -5336,7 +5590,10 @@
   ""
   "adrp\\t%0, %A1\;ldr\\t%w0, [%0, #%L1]"
   [(set_attr "type" "load1")
-   (set_attr "length" "8")]
+   (set_attr "length" "8")
+   (set_attr "update" "no")
+   (set_attr "index" "no")
+   (set_attr "index_shift" "no")]
 )
 
 (define_insn "tlsie_tiny_<mode>"
diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
index ad8a43c..df694d8 100644
--- a/gcc/config/aarch64/predicates.md
+++ b/gcc/config/aarch64/predicates.md
@@ -354,6 +354,44 @@
   return aarch64_const_vec_all_same_int_p (op, -1);
 })
 
+;; Return 1 if the operand is a MEM with an update-form address. This may
+;; also include update-indexed form.
+(define_special_predicate "update_address_mem"
+  (match_test "(MEM_P (op)
+		&& (GET_CODE (XEXP (op, 0)) == POST_INC
+		    || GET_CODE (XEXP (op, 0)) == POST_DEC
+		    || GET_CODE (XEXP (op, 0)) == POST_MODIFY
+		    || GET_CODE (XEXP (op, 0)) == PRE_INC
+		    || GET_CODE (XEXP (op, 0)) == PRE_DEC
+		    || GET_CODE (XEXP (op, 0)) == PRE_MODIFY))"))
+
+;; Return 1 if the operand is an index-form address.
+(define_special_predicate "index_address"
+  (match_test "(GET_CODE (op) == PLUS
+                && REG_P (XEXP (op, 0))
+                && REG_P (XEXP (op, 1)))"))
+
+;; Return 1 if the operand is a MEM with an indexed-form address.
+(define_special_predicate "index_address_mem"
+  (match_test "(MEM_P (op)
+                && (index_address (XEXP (op, 0), mode)
+                    || (GET_CODE (XEXP (op, 0)) == PRE_MODIFY
+                        && index_address (XEXP (XEXP (op, 0), 1), mode))))"))
+
+;; Return 1 if the operand is an index-form address.
+(define_special_predicate "index_shift_address"
+  (match_test "(GET_CODE (op) == PLUS
+                && REG_P (XEXP (op, 0))
+                && GET_CODE (XEXP (op, 1)) == ASHIFT
+                && REG_P (XEXP (XEXP (op, 1), 0)))"))
+
+;; Return 1 if the operand is a MEM with an indexed-form address.
+(define_special_predicate "index_shift_address_mem"
+  (match_test "(MEM_P (op)
+                && (index_shift_address (XEXP (op, 0), mode)
+                    || (GET_CODE (XEXP (op, 0)) == PRE_MODIFY
+                        && index_shift_address (XEXP (XEXP (op, 0), 1), mode))))"))
+
 ;; Predicates used by the various SIMD shift operations.  These
 ;; fall in to 3 categories.
 ;;   Shifts with a range 0-(bit_size - 1) (aarch64_simd_shift_imm)

  reply	other threads:[~2017-07-26  7:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-06  5:11 Hurugalawadi, Naveen
2017-04-25  7:05 ` [PING][PATCH][AArch64] " Hurugalawadi, Naveen
2017-05-11  4:47   ` [PING2][PATCH][AArch64] " Hurugalawadi, Naveen
2017-05-11  4:48   ` Hurugalawadi, Naveen
2017-05-26  6:22     ` [PING 3][PATCH][AArch64] " Hurugalawadi, Naveen
2017-05-11  4:49   ` [PING2][PATCH][AArch64] " Hurugalawadi, Naveen
2017-05-11  4:51   ` Hurugalawadi, Naveen
2017-06-14 14:39 ` [PATCH][AArch64] " James Greenhalgh
2017-07-26  7:56   ` Hurugalawadi, Naveen [this message]
2017-08-11  9:58     ` [PING] [PATCH] [AArch64] " Hurugalawadi, Naveen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CO2PR07MB26937220B9C79CDDC9E6988683B90@CO2PR07MB2693.namprd07.prod.outlook.com \
    --to=naveen.hurugalawadi@cavium.com \
    --cc=Andrew.Pinski@cavium.com \
    --cc=Richard.Earnshaw@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=james.greenhalgh@arm.com \
    --cc=marcus.shawcroft@arm.com \
    --cc=nd@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).