[AArch64] Tweak aarch64_classify_address interface

Message ID 8760dgc5wc.fsf@linaro.org
State New
Headers show

Commit Message

Richard Sandiford Aug. 22, 2017, 9:23 a.m.
Previously aarch64_classify_address used an rtx code to distinguish
LDP/STP addresses from normal addresses; the code was PARALLEL
to select LDP/STP and anything else to select normal addresses.
This patch replaces that parameter with a dedicated enum.

The SVE port will add another enum value that didn't map naturally
to an rtx code.

Tested on aarch64-linux-gnu.  OK to install?


2017-08-22  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* config/aarch64/aarch64-protos.h (aarch64_addr_query_type): New enum.
	(aarch64_legitimate_address_p): Use it instead of an rtx code.
	* config/aarch64/aarch64.c (aarch64_classify_address): Likewise.
	(aarch64_legitimate_address_p): Likewise.
	(aarch64_address_valid_for_prefetch_p): Update calls accordingly.
	(aarch64_legitimate_address_hook_p): Likewise.
	(aarch64_print_operand_address): Likewise.
	(aarch64_address_cost): Likewise.
	* config/aarch64/aarch64-simd.md (mov<mode>): Likewise.
	* config/aarch64/constraints.md (Uad): Likewise.
	* config/aarch64/predicates.md (aarch64_mem_pair_operand): Likewise.

Comments

James Greenhalgh Aug. 30, 2017, 5 p.m. | #1
On Tue, Aug 22, 2017 at 10:23:47AM +0100, Richard Sandiford wrote:
> Previously aarch64_classify_address used an rtx code to distinguish

> LDP/STP addresses from normal addresses; the code was PARALLEL

> to select LDP/STP and anything else to select normal addresses.

> This patch replaces that parameter with a dedicated enum.

> 

> The SVE port will add another enum value that didn't map naturally

> to an rtx code.

> 

> Tested on aarch64-linux-gnu.  OK to install?


I can't say I really like this new interface, I'd prefer two wrappers
aarch64_legitimate_address_p , aarch64_legitimate_ldp_address_p (or similar)
around your new interface, and for most code to simply call the wrapper.
Or an overloaded call that filled in ADDR_QUERY_M automatically, to save
that spreading through the backend.

Thanks,
James

> 

> 

> 2017-08-22  Richard Sandiford  <richard.sandiford@linaro.org>

> 	    Alan Hayward  <alan.hayward@arm.com>

> 	    David Sherwood  <david.sherwood@arm.com>

> 

> gcc/

> 	* config/aarch64/aarch64-protos.h (aarch64_addr_query_type): New enum.

> 	(aarch64_legitimate_address_p): Use it instead of an rtx code.

> 	* config/aarch64/aarch64.c (aarch64_classify_address): Likewise.

> 	(aarch64_legitimate_address_p): Likewise.

> 	(aarch64_address_valid_for_prefetch_p): Update calls accordingly.

> 	(aarch64_legitimate_address_hook_p): Likewise.

> 	(aarch64_print_operand_address): Likewise.

> 	(aarch64_address_cost): Likewise.

> 	* config/aarch64/aarch64-simd.md (mov<mode>): Likewise.

> 	* config/aarch64/constraints.md (Uad): Likewise.

> 	* config/aarch64/predicates.md (aarch64_mem_pair_operand): Likewise.
Richard Sandiford Aug. 31, 2017, 10:27 a.m. | #2
James Greenhalgh <james.greenhalgh@arm.com> writes:
> On Tue, Aug 22, 2017 at 10:23:47AM +0100, Richard Sandiford wrote:

>> Previously aarch64_classify_address used an rtx code to distinguish

>> LDP/STP addresses from normal addresses; the code was PARALLEL

>> to select LDP/STP and anything else to select normal addresses.

>> This patch replaces that parameter with a dedicated enum.

>> 

>> The SVE port will add another enum value that didn't map naturally

>> to an rtx code.

>> 

>> Tested on aarch64-linux-gnu.  OK to install?

>

> I can't say I really like this new interface, I'd prefer two wrappers

> aarch64_legitimate_address_p , aarch64_legitimate_ldp_address_p (or similar)

> around your new interface, and for most code to simply call the wrapper.

> Or an overloaded call that filled in ADDR_QUERY_M automatically, to save

> that spreading through the backend.


OK, I went for the second, putting the query type last and making it
an optional argument.

Tested on aarch64-linux-gnu.  OK to install?

Thanks,
Richard


2017-08-31  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* config/aarch64/aarch64-protos.h (aarch64_addr_query_type): New enum.
	(aarch64_legitimate_address_p): Use it instead of an rtx code,
	as an optional final parameter.
	* config/aarch64/aarch64.c (aarch64_classify_address): Likewise.
	(aarch64_legitimate_address_p): Likewise.
	(aarch64_address_valid_for_prefetch_p): Update calls accordingly.
	(aarch64_legitimate_address_hook_p): Likewise.
	(aarch64_print_operand_address): Likewise.
	(aarch64_address_cost): Likewise.
	* config/aarch64/aarch64-simd.md (mov<mode>): Likewise.
	* config/aarch64/constraints.md (Uad): Likewise.
	* config/aarch64/predicates.md (aarch64_mem_pair_operand): Likewise.

Index: gcc/config/aarch64/aarch64-protos.h
===================================================================
--- gcc/config/aarch64/aarch64-protos.h	2017-08-29 20:01:08.126372143 +0100
+++ gcc/config/aarch64/aarch64-protos.h	2017-08-31 10:34:44.007238668 +0100
@@ -111,6 +111,19 @@ enum aarch64_symbol_type
   SYMBOL_FORCE_TO_MEM
 };
 
+/* Classifies the type of an address query.
+
+   ADDR_QUERY_M
+      Query what is valid for an "m" constraint and a memory_operand
+      (the rules are the same for both).
+
+   ADDR_QUERY_LDP_STP
+      Query what is valid for a load/store pair.  */
+enum aarch64_addr_query_type {
+  ADDR_QUERY_M,
+  ADDR_QUERY_LDP_STP
+};
+
 /* A set of tuning parameters contains references to size and time
    cost models and vectors for address cost calculations, register
    move costs and memory move costs.  */
@@ -433,7 +446,8 @@ bool aarch64_float_const_representable_p
 
 #if defined (RTX_CODE)
 
-bool aarch64_legitimate_address_p (machine_mode, rtx, RTX_CODE, bool);
+bool aarch64_legitimate_address_p (machine_mode, rtx, bool,
+				   aarch64_addr_query_type = ADDR_QUERY_M);
 machine_mode aarch64_select_cc_mode (RTX_CODE, rtx, rtx);
 rtx aarch64_gen_compare_reg (RTX_CODE, rtx, rtx);
 rtx aarch64_load_tp (rtx);
Index: gcc/config/aarch64/aarch64.c
===================================================================
--- gcc/config/aarch64/aarch64.c	2017-08-30 12:20:31.681622403 +0100
+++ gcc/config/aarch64/aarch64.c	2017-08-31 10:34:44.009238668 +0100
@@ -4385,21 +4385,21 @@ virt_or_elim_regno_p (unsigned regno)
 	  || regno == ARG_POINTER_REGNUM);
 }
 
-/* Return true if X is a valid address for machine mode MODE.  If it is,
-   fill in INFO appropriately.  STRICT_P is true if REG_OK_STRICT is in
-   effect.  OUTER_CODE is PARALLEL for a load/store pair.  */
+/* Return true if X is a valid address of type TYPE for machine mode MODE.
+   If it is, fill in INFO appropriately.  STRICT_P is true if
+   REG_OK_STRICT is in effect.  */
 
 static bool
 aarch64_classify_address (struct aarch64_address_info *info,
-			  rtx x, machine_mode mode,
-			  RTX_CODE outer_code, bool strict_p)
+			  rtx x, machine_mode mode, bool strict_p,
+			  aarch64_addr_query_type type = ADDR_QUERY_M)
 {
   enum rtx_code code = GET_CODE (x);
   rtx op0, op1;
 
   /* On BE, we use load/store pair for all large int mode load/stores.
      TI/TFmode may also use a load/store pair.  */
-  bool load_store_pair_p = (outer_code == PARALLEL
+  bool load_store_pair_p = (type == ADDR_QUERY_LDP_STP
 			    || mode == TImode
 			    || mode == TFmode
 			    || (BYTES_BIG_ENDIAN
@@ -4631,7 +4631,7 @@ aarch64_address_valid_for_prefetch_p (rt
   struct aarch64_address_info addr;
 
   /* PRFM accepts the same addresses as DImode...  */
-  bool res = aarch64_classify_address (&addr, x, DImode, MEM, strict_p);
+  bool res = aarch64_classify_address (&addr, x, DImode, strict_p);
   if (!res)
     return false;
 
@@ -4667,19 +4667,18 @@ aarch64_legitimate_address_hook_p (machi
 {
   struct aarch64_address_info addr;
 
-  return aarch64_classify_address (&addr, x, mode, MEM, strict_p);
+  return aarch64_classify_address (&addr, x, mode, strict_p);
 }
 
-/* Return TRUE if X is a legitimate address for accessing memory in
-   mode MODE.  OUTER_CODE will be PARALLEL if this is a load/store
-   pair operation.  */
+/* Return TRUE if X is a legitimate address of type TYPE for accessing
+   memory in mode MODE.  STRICT_P is true if REG_OK_STRICT is in effect.  */
 bool
-aarch64_legitimate_address_p (machine_mode mode, rtx x,
-			      RTX_CODE outer_code, bool strict_p)
+aarch64_legitimate_address_p (machine_mode mode, rtx x, bool strict_p,
+			      aarch64_addr_query_type type)
 {
   struct aarch64_address_info addr;
 
-  return aarch64_classify_address (&addr, x, mode, outer_code, strict_p);
+  return aarch64_classify_address (&addr, x, mode, strict_p, type);
 }
 
 /* Split an out-of-range address displacement into a base and offset.
@@ -5552,7 +5551,7 @@ aarch64_print_operand_address (FILE *f,
 {
   struct aarch64_address_info addr;
 
-  if (aarch64_classify_address (&addr, x, mode, MEM, true))
+  if (aarch64_classify_address (&addr, x, mode, true))
     switch (addr.type)
       {
       case ADDRESS_REG_IMM:
@@ -6476,7 +6475,7 @@ aarch64_address_cost (rtx x,
   int cost = 0;
   info.shift = 0;
 
-  if (!aarch64_classify_address (&info, x, mode, c, false))
+  if (!aarch64_classify_address (&info, x, mode, false))
     {
       if (GET_CODE (x) == CONST || GET_CODE (x) == SYMBOL_REF)
 	{
Index: gcc/config/aarch64/aarch64-simd.md
===================================================================
--- gcc/config/aarch64/aarch64-simd.md	2017-08-31 10:34:35.905238126 +0100
+++ gcc/config/aarch64/aarch64-simd.md	2017-08-31 10:34:44.008238668 +0100
@@ -25,8 +25,8 @@ (define_expand "mov<mode>"
   "
     if (GET_CODE (operands[0]) == MEM
 	&& !(aarch64_simd_imm_zero (operands[1], <MODE>mode)
-	     && aarch64_legitimate_address_p (<MODE>mode, operands[0],
-					      PARALLEL, 1)))
+	     && aarch64_legitimate_address_p (<MODE>mode, operands[0], true,
+					      ADDR_QUERY_LDP_STP)))
       operands[1] = force_reg (<MODE>mode, operands[1]);
   "
 )
Index: gcc/config/aarch64/constraints.md
===================================================================
--- gcc/config/aarch64/constraints.md	2017-08-29 20:01:06.951372082 +0100
+++ gcc/config/aarch64/constraints.md	2017-08-31 10:34:44.010238668 +0100
@@ -161,7 +161,7 @@ (define_memory_constraint "Ump"
   A memory address suitable for a load/store pair operation."
   (and (match_code "mem")
        (match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP (op, 0),
-						  PARALLEL, 1)")))
+						  true, ADDR_QUERY_LDP_STP)")))
 
 (define_memory_constraint "Utv"
   "@internal
Index: gcc/config/aarch64/predicates.md
===================================================================
--- gcc/config/aarch64/predicates.md	2017-08-29 20:01:06.951372082 +0100
+++ gcc/config/aarch64/predicates.md	2017-08-31 10:34:44.010238668 +0100
@@ -174,8 +174,8 @@ (define_predicate "aarch64_mem_pair_offs
 
 (define_predicate "aarch64_mem_pair_operand"
   (and (match_code "mem")
-       (match_test "aarch64_legitimate_address_p (mode, XEXP (op, 0), PARALLEL,
-					       0)")))
+       (match_test "aarch64_legitimate_address_p (mode, XEXP (op, 0), false,
+						  ADDR_QUERY_LDP_STP)")))
 
 (define_predicate "aarch64_prefetch_operand"
   (match_test "aarch64_address_valid_for_prefetch_p (op, false)"))
Richard Sandiford Sept. 18, 2017, 1:43 p.m. | #3
Richard Sandiford <richard.sandiford@linaro.org> writes:
> James Greenhalgh <james.greenhalgh@arm.com> writes:

>> On Tue, Aug 22, 2017 at 10:23:47AM +0100, Richard Sandiford wrote:

>>> Previously aarch64_classify_address used an rtx code to distinguish

>>> LDP/STP addresses from normal addresses; the code was PARALLEL

>>> to select LDP/STP and anything else to select normal addresses.

>>> This patch replaces that parameter with a dedicated enum.

>>> 

>>> The SVE port will add another enum value that didn't map naturally

>>> to an rtx code.

>>> 

>>> Tested on aarch64-linux-gnu.  OK to install?

>>

>> I can't say I really like this new interface, I'd prefer two wrappers

>> aarch64_legitimate_address_p , aarch64_legitimate_ldp_address_p (or similar)

>> around your new interface, and for most code to simply call the wrapper.

>> Or an overloaded call that filled in ADDR_QUERY_M automatically, to save

>> that spreading through the backend.

>

> OK, I went for the second, putting the query type last and making it

> an optional argument.


By way of a ping, here's the patch updated to current trunk.

Tested on aarch64-linux-gnu.  OK to install?

Thanks,
Richard

2017-09-18  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* config/aarch64/aarch64-protos.h (aarch64_addr_query_type): New enum.
	(aarch64_legitimate_address_p): Use it instead of an rtx code,
	as an optional final parameter.
	* config/aarch64/aarch64.c (aarch64_classify_address): Likewise.
	(aarch64_legitimate_address_p): Likewise.
	(aarch64_address_valid_for_prefetch_p): Update calls accordingly.
	(aarch64_legitimate_address_hook_p): Likewise.
	(aarch64_print_operand_address): Likewise.
	(aarch64_address_cost): Likewise.
	* config/aarch64/constraints.md (Umq, Ump): Likewise.
	* config/aarch64/predicates.md (aarch64_mem_pair_operand): Likewise.

Index: gcc/config/aarch64/aarch64-protos.h
===================================================================
--- gcc/config/aarch64/aarch64-protos.h	2017-09-18 14:41:37.369070450 +0100
+++ gcc/config/aarch64/aarch64-protos.h	2017-09-18 14:42:29.656488378 +0100
@@ -111,6 +111,19 @@ enum aarch64_symbol_type
   SYMBOL_FORCE_TO_MEM
 };
 
+/* Classifies the type of an address query.
+
+   ADDR_QUERY_M
+      Query what is valid for an "m" constraint and a memory_operand
+      (the rules are the same for both).
+
+   ADDR_QUERY_LDP_STP
+      Query what is valid for a load/store pair.  */
+enum aarch64_addr_query_type {
+  ADDR_QUERY_M,
+  ADDR_QUERY_LDP_STP
+};
+
 /* A set of tuning parameters contains references to size and time
    cost models and vectors for address cost calculations, register
    move costs and memory move costs.  */
@@ -427,7 +440,8 @@ bool aarch64_float_const_representable_p
 
 #if defined (RTX_CODE)
 
-bool aarch64_legitimate_address_p (machine_mode, rtx, RTX_CODE, bool);
+bool aarch64_legitimate_address_p (machine_mode, rtx, bool,
+				   aarch64_addr_query_type = ADDR_QUERY_M);
 machine_mode aarch64_select_cc_mode (RTX_CODE, rtx, rtx);
 rtx aarch64_gen_compare_reg (RTX_CODE, rtx, rtx);
 rtx aarch64_load_tp (rtx);
Index: gcc/config/aarch64/aarch64.c
===================================================================
--- gcc/config/aarch64/aarch64.c	2017-09-18 14:41:37.373588926 +0100
+++ gcc/config/aarch64/aarch64.c	2017-09-18 14:42:29.657389742 +0100
@@ -4409,21 +4409,21 @@ virt_or_elim_regno_p (unsigned regno)
 	  || regno == ARG_POINTER_REGNUM);
 }
 
-/* Return true if X is a valid address for machine mode MODE.  If it is,
-   fill in INFO appropriately.  STRICT_P is true if REG_OK_STRICT is in
-   effect.  OUTER_CODE is PARALLEL for a load/store pair.  */
+/* Return true if X is a valid address of type TYPE for machine mode MODE.
+   If it is, fill in INFO appropriately.  STRICT_P is true if
+   REG_OK_STRICT is in effect.  */
 
 static bool
 aarch64_classify_address (struct aarch64_address_info *info,
-			  rtx x, machine_mode mode,
-			  RTX_CODE outer_code, bool strict_p)
+			  rtx x, machine_mode mode, bool strict_p,
+			  aarch64_addr_query_type type = ADDR_QUERY_M)
 {
   enum rtx_code code = GET_CODE (x);
   rtx op0, op1;
 
   /* On BE, we use load/store pair for all large int mode load/stores.
      TI/TFmode may also use a load/store pair.  */
-  bool load_store_pair_p = (outer_code == PARALLEL
+  bool load_store_pair_p = (type == ADDR_QUERY_LDP_STP
 			    || mode == TImode
 			    || mode == TFmode
 			    || (BYTES_BIG_ENDIAN
@@ -4655,7 +4655,7 @@ aarch64_address_valid_for_prefetch_p (rt
   struct aarch64_address_info addr;
 
   /* PRFM accepts the same addresses as DImode...  */
-  bool res = aarch64_classify_address (&addr, x, DImode, MEM, strict_p);
+  bool res = aarch64_classify_address (&addr, x, DImode, strict_p);
   if (!res)
     return false;
 
@@ -4691,19 +4691,18 @@ aarch64_legitimate_address_hook_p (machi
 {
   struct aarch64_address_info addr;
 
-  return aarch64_classify_address (&addr, x, mode, MEM, strict_p);
+  return aarch64_classify_address (&addr, x, mode, strict_p);
 }
 
-/* Return TRUE if X is a legitimate address for accessing memory in
-   mode MODE.  OUTER_CODE will be PARALLEL if this is a load/store
-   pair operation.  */
+/* Return TRUE if X is a legitimate address of type TYPE for accessing
+   memory in mode MODE.  STRICT_P is true if REG_OK_STRICT is in effect.  */
 bool
-aarch64_legitimate_address_p (machine_mode mode, rtx x,
-			      RTX_CODE outer_code, bool strict_p)
+aarch64_legitimate_address_p (machine_mode mode, rtx x, bool strict_p,
+			      aarch64_addr_query_type type)
 {
   struct aarch64_address_info addr;
 
-  return aarch64_classify_address (&addr, x, mode, outer_code, strict_p);
+  return aarch64_classify_address (&addr, x, mode, strict_p, type);
 }
 
 /* Split an out-of-range address displacement into a base and offset.
@@ -5574,7 +5573,7 @@ aarch64_print_operand_address (FILE *f,
 {
   struct aarch64_address_info addr;
 
-  if (aarch64_classify_address (&addr, x, mode, MEM, true))
+  if (aarch64_classify_address (&addr, x, mode, true))
     switch (addr.type)
       {
       case ADDRESS_REG_IMM:
@@ -6503,7 +6502,7 @@ aarch64_address_cost (rtx x,
   int cost = 0;
   info.shift = 0;
 
-  if (!aarch64_classify_address (&info, x, mode, c, false))
+  if (!aarch64_classify_address (&info, x, mode, false))
     {
       if (GET_CODE (x) == CONST || GET_CODE (x) == SYMBOL_REF)
 	{
Index: gcc/config/aarch64/constraints.md
===================================================================
--- gcc/config/aarch64/constraints.md	2017-09-18 14:41:37.374492621 +0100
+++ gcc/config/aarch64/constraints.md	2017-09-18 14:42:29.657389742 +0100
@@ -161,15 +161,15 @@ (define_memory_constraint "Umq"
    A memory address which uses a base register with an offset small enough for
    a load/store pair operation in DI mode."
    (and (match_code "mem")
-	(match_test "aarch64_legitimate_address_p (DImode, XEXP (op, 0),
-						   PARALLEL, false)")))
+	(match_test "aarch64_legitimate_address_p (DImode, XEXP (op, 0), false,
+						   ADDR_QUERY_LDP_STP)")))
 
 (define_memory_constraint "Ump"
   "@internal
   A memory address suitable for a load/store pair operation."
   (and (match_code "mem")
        (match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP (op, 0),
-						  PARALLEL, 1)")))
+						  true, ADDR_QUERY_LDP_STP)")))
 
 (define_memory_constraint "Utv"
   "@internal
Index: gcc/config/aarch64/predicates.md
===================================================================
--- gcc/config/aarch64/predicates.md	2017-09-18 14:41:37.374492621 +0100
+++ gcc/config/aarch64/predicates.md	2017-09-18 14:42:29.657389742 +0100
@@ -174,8 +174,8 @@ (define_predicate "aarch64_mem_pair_offs
 
 (define_predicate "aarch64_mem_pair_operand"
   (and (match_code "mem")
-       (match_test "aarch64_legitimate_address_p (mode, XEXP (op, 0), PARALLEL,
-					       0)")))
+       (match_test "aarch64_legitimate_address_p (mode, XEXP (op, 0), false,
+						  ADDR_QUERY_LDP_STP)")))
 
 (define_predicate "aarch64_prefetch_operand"
   (match_test "aarch64_address_valid_for_prefetch_p (op, false)"))
Richard Sandiford Oct. 23, 2017, 5:58 p.m. | #4
Ping.

Richard Sandiford <richard.sandiford@linaro.org> writes:
> Richard Sandiford <richard.sandiford@linaro.org> writes:

>> James Greenhalgh <james.greenhalgh@arm.com> writes:

>>> On Tue, Aug 22, 2017 at 10:23:47AM +0100, Richard Sandiford wrote:

>>>> Previously aarch64_classify_address used an rtx code to distinguish

>>>> LDP/STP addresses from normal addresses; the code was PARALLEL

>>>> to select LDP/STP and anything else to select normal addresses.

>>>> This patch replaces that parameter with a dedicated enum.

>>>> 

>>>> The SVE port will add another enum value that didn't map naturally

>>>> to an rtx code.

>>>> 

>>>> Tested on aarch64-linux-gnu.  OK to install?

>>>

>>> I can't say I really like this new interface, I'd prefer two wrappers

>>> aarch64_legitimate_address_p , aarch64_legitimate_ldp_address_p (or similar)

>>> around your new interface, and for most code to simply call the wrapper.

>>> Or an overloaded call that filled in ADDR_QUERY_M automatically, to save

>>> that spreading through the backend.

>>

>> OK, I went for the second, putting the query type last and making it

>> an optional argument.

>

> By way of a ping, here's the patch updated to current trunk.

>

> Tested on aarch64-linux-gnu.  OK to install?

>

> Thanks,

> Richard

>

> 2017-09-18  Richard Sandiford  <richard.sandiford@linaro.org>

> 	    Alan Hayward  <alan.hayward@arm.com>

> 	    David Sherwood  <david.sherwood@arm.com>

>

> gcc/

> 	* config/aarch64/aarch64-protos.h (aarch64_addr_query_type): New enum.

> 	(aarch64_legitimate_address_p): Use it instead of an rtx code,

> 	as an optional final parameter.

> 	* config/aarch64/aarch64.c (aarch64_classify_address): Likewise.

> 	(aarch64_legitimate_address_p): Likewise.

> 	(aarch64_address_valid_for_prefetch_p): Update calls accordingly.

> 	(aarch64_legitimate_address_hook_p): Likewise.

> 	(aarch64_print_operand_address): Likewise.

> 	(aarch64_address_cost): Likewise.

> 	* config/aarch64/constraints.md (Umq, Ump): Likewise.

> 	* config/aarch64/predicates.md (aarch64_mem_pair_operand): Likewise.

>

> Index: gcc/config/aarch64/aarch64-protos.h

> ===================================================================

> --- gcc/config/aarch64/aarch64-protos.h	2017-09-18 14:41:37.369070450 +0100

> +++ gcc/config/aarch64/aarch64-protos.h	2017-09-18 14:42:29.656488378 +0100

> @@ -111,6 +111,19 @@ enum aarch64_symbol_type

>    SYMBOL_FORCE_TO_MEM

>  };

>  

> +/* Classifies the type of an address query.

> +

> +   ADDR_QUERY_M

> +      Query what is valid for an "m" constraint and a memory_operand

> +      (the rules are the same for both).

> +

> +   ADDR_QUERY_LDP_STP

> +      Query what is valid for a load/store pair.  */

> +enum aarch64_addr_query_type {

> +  ADDR_QUERY_M,

> +  ADDR_QUERY_LDP_STP

> +};

> +

>  /* A set of tuning parameters contains references to size and time

>     cost models and vectors for address cost calculations, register

>     move costs and memory move costs.  */

> @@ -427,7 +440,8 @@ bool aarch64_float_const_representable_p

>  

>  #if defined (RTX_CODE)

>  

> -bool aarch64_legitimate_address_p (machine_mode, rtx, RTX_CODE, bool);

> +bool aarch64_legitimate_address_p (machine_mode, rtx, bool,

> +				   aarch64_addr_query_type = ADDR_QUERY_M);

>  machine_mode aarch64_select_cc_mode (RTX_CODE, rtx, rtx);

>  rtx aarch64_gen_compare_reg (RTX_CODE, rtx, rtx);

>  rtx aarch64_load_tp (rtx);

> Index: gcc/config/aarch64/aarch64.c

> ===================================================================

> --- gcc/config/aarch64/aarch64.c	2017-09-18 14:41:37.373588926 +0100

> +++ gcc/config/aarch64/aarch64.c	2017-09-18 14:42:29.657389742 +0100

> @@ -4409,21 +4409,21 @@ virt_or_elim_regno_p (unsigned regno)

>  	  || regno == ARG_POINTER_REGNUM);

>  }

>  

> -/* Return true if X is a valid address for machine mode MODE.  If it is,

> -   fill in INFO appropriately.  STRICT_P is true if REG_OK_STRICT is in

> -   effect.  OUTER_CODE is PARALLEL for a load/store pair.  */

> +/* Return true if X is a valid address of type TYPE for machine mode MODE.

> +   If it is, fill in INFO appropriately.  STRICT_P is true if

> +   REG_OK_STRICT is in effect.  */

>  

>  static bool

>  aarch64_classify_address (struct aarch64_address_info *info,

> -			  rtx x, machine_mode mode,

> -			  RTX_CODE outer_code, bool strict_p)

> +			  rtx x, machine_mode mode, bool strict_p,

> +			  aarch64_addr_query_type type = ADDR_QUERY_M)

>  {

>    enum rtx_code code = GET_CODE (x);

>    rtx op0, op1;

>  

>    /* On BE, we use load/store pair for all large int mode load/stores.

>       TI/TFmode may also use a load/store pair.  */

> -  bool load_store_pair_p = (outer_code == PARALLEL

> +  bool load_store_pair_p = (type == ADDR_QUERY_LDP_STP

>  			    || mode == TImode

>  			    || mode == TFmode

>  			    || (BYTES_BIG_ENDIAN

> @@ -4655,7 +4655,7 @@ aarch64_address_valid_for_prefetch_p (rt

>    struct aarch64_address_info addr;

>  

>    /* PRFM accepts the same addresses as DImode...  */

> -  bool res = aarch64_classify_address (&addr, x, DImode, MEM, strict_p);

> +  bool res = aarch64_classify_address (&addr, x, DImode, strict_p);

>    if (!res)

>      return false;

>  

> @@ -4691,19 +4691,18 @@ aarch64_legitimate_address_hook_p (machi

>  {

>    struct aarch64_address_info addr;

>  

> -  return aarch64_classify_address (&addr, x, mode, MEM, strict_p);

> +  return aarch64_classify_address (&addr, x, mode, strict_p);

>  }

>  

> -/* Return TRUE if X is a legitimate address for accessing memory in

> -   mode MODE.  OUTER_CODE will be PARALLEL if this is a load/store

> -   pair operation.  */

> +/* Return TRUE if X is a legitimate address of type TYPE for accessing

> +   memory in mode MODE.  STRICT_P is true if REG_OK_STRICT is in effect.  */

>  bool

> -aarch64_legitimate_address_p (machine_mode mode, rtx x,

> -			      RTX_CODE outer_code, bool strict_p)

> +aarch64_legitimate_address_p (machine_mode mode, rtx x, bool strict_p,

> +			      aarch64_addr_query_type type)

>  {

>    struct aarch64_address_info addr;

>  

> -  return aarch64_classify_address (&addr, x, mode, outer_code, strict_p);

> +  return aarch64_classify_address (&addr, x, mode, strict_p, type);

>  }

>  

>  /* Split an out-of-range address displacement into a base and offset.

> @@ -5574,7 +5573,7 @@ aarch64_print_operand_address (FILE *f,

>  {

>    struct aarch64_address_info addr;

>  

> -  if (aarch64_classify_address (&addr, x, mode, MEM, true))

> +  if (aarch64_classify_address (&addr, x, mode, true))

>      switch (addr.type)

>        {

>        case ADDRESS_REG_IMM:

> @@ -6503,7 +6502,7 @@ aarch64_address_cost (rtx x,

>    int cost = 0;

>    info.shift = 0;

>  

> -  if (!aarch64_classify_address (&info, x, mode, c, false))

> +  if (!aarch64_classify_address (&info, x, mode, false))

>      {

>        if (GET_CODE (x) == CONST || GET_CODE (x) == SYMBOL_REF)

>  	{

> Index: gcc/config/aarch64/constraints.md

> ===================================================================

> --- gcc/config/aarch64/constraints.md	2017-09-18 14:41:37.374492621 +0100

> +++ gcc/config/aarch64/constraints.md	2017-09-18 14:42:29.657389742 +0100

> @@ -161,15 +161,15 @@ (define_memory_constraint "Umq"

>     A memory address which uses a base register with an offset small enough for

>     a load/store pair operation in DI mode."

>     (and (match_code "mem")

> -	(match_test "aarch64_legitimate_address_p (DImode, XEXP (op, 0),

> -						   PARALLEL, false)")))

> +	(match_test "aarch64_legitimate_address_p (DImode, XEXP (op, 0), false,

> +						   ADDR_QUERY_LDP_STP)")))

>  

>  (define_memory_constraint "Ump"

>    "@internal

>    A memory address suitable for a load/store pair operation."

>    (and (match_code "mem")

>         (match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP (op, 0),

> -						  PARALLEL, 1)")))

> +						  true, ADDR_QUERY_LDP_STP)")))

>  

>  (define_memory_constraint "Utv"

>    "@internal

> Index: gcc/config/aarch64/predicates.md

> ===================================================================

> --- gcc/config/aarch64/predicates.md	2017-09-18 14:41:37.374492621 +0100

> +++ gcc/config/aarch64/predicates.md	2017-09-18 14:42:29.657389742 +0100

> @@ -174,8 +174,8 @@ (define_predicate "aarch64_mem_pair_offs

>  

>  (define_predicate "aarch64_mem_pair_operand"

>    (and (match_code "mem")

> -       (match_test "aarch64_legitimate_address_p (mode, XEXP (op, 0), PARALLEL,

> -					       0)")))

> +       (match_test "aarch64_legitimate_address_p (mode, XEXP (op, 0), false,

> +						  ADDR_QUERY_LDP_STP)")))

>  

>  (define_predicate "aarch64_prefetch_operand"

>    (match_test "aarch64_address_valid_for_prefetch_p (op, false)"))
James Greenhalgh Nov. 10, 2017, 3:16 p.m. | #5
On Mon, Oct 23, 2017 at 06:58:29PM +0100, Richard Sandiford wrote:
> Ping.


Makes sense. OK.

Reviewed-By: James Greenhalgh <james.greenhalgh@arm.com>


James

> Richard Sandiford <richard.sandiford@linaro.org> writes:

> > Richard Sandiford <richard.sandiford@linaro.org> writes:

> >> James Greenhalgh <james.greenhalgh@arm.com> writes:

> >>> On Tue, Aug 22, 2017 at 10:23:47AM +0100, Richard Sandiford wrote:

> >>>> Previously aarch64_classify_address used an rtx code to distinguish

> >>>> LDP/STP addresses from normal addresses; the code was PARALLEL

> >>>> to select LDP/STP and anything else to select normal addresses.

> >>>> This patch replaces that parameter with a dedicated enum.

> >>>> 

> >>>> The SVE port will add another enum value that didn't map naturally

> >>>> to an rtx code.

> >>>> 

> >>>> Tested on aarch64-linux-gnu.  OK to install?

> >>>

> >>> I can't say I really like this new interface, I'd prefer two wrappers

> >>> aarch64_legitimate_address_p , aarch64_legitimate_ldp_address_p (or similar)

> >>> around your new interface, and for most code to simply call the wrapper.

> >>> Or an overloaded call that filled in ADDR_QUERY_M automatically, to save

> >>> that spreading through the backend.

> >>

> >> OK, I went for the second, putting the query type last and making it

> >> an optional argument.

> >

> > By way of a ping, here's the patch updated to current trunk.

> >

> > Tested on aarch64-linux-gnu.  OK to install?

> >

> > Thanks,

> > Richard

> >

> > 2017-09-18  Richard Sandiford  <richard.sandiford@linaro.org>

> > 	    Alan Hayward  <alan.hayward@arm.com>

> > 	    David Sherwood  <david.sherwood@arm.com>

> >

> > gcc/

> > 	* config/aarch64/aarch64-protos.h (aarch64_addr_query_type): New enum.

> > 	(aarch64_legitimate_address_p): Use it instead of an rtx code,

> > 	as an optional final parameter.

> > 	* config/aarch64/aarch64.c (aarch64_classify_address): Likewise.

> > 	(aarch64_legitimate_address_p): Likewise.

> > 	(aarch64_address_valid_for_prefetch_p): Update calls accordingly.

> > 	(aarch64_legitimate_address_hook_p): Likewise.

> > 	(aarch64_print_operand_address): Likewise.

> > 	(aarch64_address_cost): Likewise.

> > 	* config/aarch64/constraints.md (Umq, Ump): Likewise.

> > 	* config/aarch64/predicates.md (aarch64_mem_pair_operand): Likewise.

> >

Patch

Index: gcc/config/aarch64/aarch64-protos.h
===================================================================
--- gcc/config/aarch64/aarch64-protos.h	2017-08-03 10:40:55.900281836 +0100
+++ gcc/config/aarch64/aarch64-protos.h	2017-08-22 10:12:29.278432707 +0100
@@ -111,6 +111,19 @@  enum aarch64_symbol_type
   SYMBOL_FORCE_TO_MEM
 };
 
+/* Classifies the type of an address query.
+
+   ADDR_QUERY_M
+      Query what is valid for an "m" constraint and a memory_operand
+      (the rules are the same for both).
+
+   ADDR_QUERY_LDP_STP
+      Query what is valid for a load/store pair.  */
+enum aarch64_addr_query_type {
+  ADDR_QUERY_M,
+  ADDR_QUERY_LDP_STP
+};
+
 /* A set of tuning parameters contains references to size and time
    cost models and vectors for address cost calculations, register
    move costs and memory move costs.  */
@@ -433,7 +446,8 @@  bool aarch64_float_const_representable_p
 
 #if defined (RTX_CODE)
 
-bool aarch64_legitimate_address_p (machine_mode, rtx, RTX_CODE, bool);
+bool aarch64_legitimate_address_p (machine_mode, rtx,
+				   aarch64_addr_query_type, bool);
 machine_mode aarch64_select_cc_mode (RTX_CODE, rtx, rtx);
 rtx aarch64_gen_compare_reg (RTX_CODE, rtx, rtx);
 rtx aarch64_load_tp (rtx);
Index: gcc/config/aarch64/aarch64.c
===================================================================
--- gcc/config/aarch64/aarch64.c	2017-08-10 14:36:08.826443491 +0100
+++ gcc/config/aarch64/aarch64.c	2017-08-22 10:12:29.280432708 +0100
@@ -4385,21 +4385,21 @@  virt_or_elim_regno_p (unsigned regno)
 	  || regno == ARG_POINTER_REGNUM);
 }
 
-/* Return true if X is a valid address for machine mode MODE.  If it is,
-   fill in INFO appropriately.  STRICT_P is true if REG_OK_STRICT is in
-   effect.  OUTER_CODE is PARALLEL for a load/store pair.  */
+/* Return true if X is a valid address of type TYPE for machine mode MODE.
+   If it is, fill in INFO appropriately.  STRICT_P is true if
+   REG_OK_STRICT is in effect.  */
 
 static bool
 aarch64_classify_address (struct aarch64_address_info *info,
 			  rtx x, machine_mode mode,
-			  RTX_CODE outer_code, bool strict_p)
+			  aarch64_addr_query_type type, bool strict_p)
 {
   enum rtx_code code = GET_CODE (x);
   rtx op0, op1;
 
   /* On BE, we use load/store pair for all large int mode load/stores.
      TI/TFmode may also use a load/store pair.  */
-  bool load_store_pair_p = (outer_code == PARALLEL
+  bool load_store_pair_p = (type == ADDR_QUERY_LDP_STP
 			    || mode == TImode
 			    || mode == TFmode
 			    || (BYTES_BIG_ENDIAN
@@ -4631,7 +4631,8 @@  aarch64_address_valid_for_prefetch_p (rt
   struct aarch64_address_info addr;
 
   /* PRFM accepts the same addresses as DImode...  */
-  bool res = aarch64_classify_address (&addr, x, DImode, MEM, strict_p);
+  bool res = aarch64_classify_address (&addr, x, DImode, ADDR_QUERY_M,
+				       strict_p);
   if (!res)
     return false;
 
@@ -4667,19 +4668,18 @@  aarch64_legitimate_address_hook_p (machi
 {
   struct aarch64_address_info addr;
 
-  return aarch64_classify_address (&addr, x, mode, MEM, strict_p);
+  return aarch64_classify_address (&addr, x, mode, ADDR_QUERY_M, strict_p);
 }
 
-/* Return TRUE if X is a legitimate address for accessing memory in
-   mode MODE.  OUTER_CODE will be PARALLEL if this is a load/store
-   pair operation.  */
+/* Return TRUE if X is a legitimate address of type TYPE for accessing
+   memory in mode MODE.  STRICT_P is true if REG_OK_STRICT is in effect.  */
 bool
 aarch64_legitimate_address_p (machine_mode mode, rtx x,
-			      RTX_CODE outer_code, bool strict_p)
+			      aarch64_addr_query_type type, bool strict_p)
 {
   struct aarch64_address_info addr;
 
-  return aarch64_classify_address (&addr, x, mode, outer_code, strict_p);
+  return aarch64_classify_address (&addr, x, mode, type, strict_p);
 }
 
 /* Split an out-of-range address displacement into a base and offset.
@@ -5550,7 +5550,7 @@  aarch64_print_operand_address (FILE *f,
 {
   struct aarch64_address_info addr;
 
-  if (aarch64_classify_address (&addr, x, mode, MEM, true))
+  if (aarch64_classify_address (&addr, x, mode, ADDR_QUERY_M, true))
     switch (addr.type)
       {
       case ADDRESS_REG_IMM:
@@ -6474,7 +6474,7 @@  aarch64_address_cost (rtx x,
   int cost = 0;
   info.shift = 0;
 
-  if (!aarch64_classify_address (&info, x, mode, c, false))
+  if (!aarch64_classify_address (&info, x, mode, ADDR_QUERY_M, false))
     {
       if (GET_CODE (x) == CONST || GET_CODE (x) == SYMBOL_REF)
 	{
Index: gcc/config/aarch64/aarch64-simd.md
===================================================================
--- gcc/config/aarch64/aarch64-simd.md	2017-08-22 10:11:45.066232915 +0100
+++ gcc/config/aarch64/aarch64-simd.md	2017-08-22 10:12:29.279432707 +0100
@@ -26,7 +26,7 @@  (define_expand "mov<mode>"
     if (GET_CODE (operands[0]) == MEM
 	&& !(aarch64_simd_imm_zero (operands[1], <MODE>mode)
 	     && aarch64_legitimate_address_p (<MODE>mode, operands[0],
-					      PARALLEL, 1)))
+					      ADDR_QUERY_LDP_STP, 1)))
       operands[1] = force_reg (<MODE>mode, operands[1]);
   "
 )
Index: gcc/config/aarch64/constraints.md
===================================================================
--- gcc/config/aarch64/constraints.md	2017-08-03 10:40:55.900281836 +0100
+++ gcc/config/aarch64/constraints.md	2017-08-22 10:12:29.280432708 +0100
@@ -161,7 +161,7 @@  (define_memory_constraint "Ump"
   A memory address suitable for a load/store pair operation."
   (and (match_code "mem")
        (match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP (op, 0),
-						  PARALLEL, 1)")))
+						  ADDR_QUERY_LDP_STP, 1)")))
 
 (define_memory_constraint "Utv"
   "@internal
Index: gcc/config/aarch64/predicates.md
===================================================================
--- gcc/config/aarch64/predicates.md	2017-08-16 08:50:34.059622539 +0100
+++ gcc/config/aarch64/predicates.md	2017-08-22 10:12:29.281432708 +0100
@@ -174,8 +174,8 @@  (define_predicate "aarch64_mem_pair_offs
 
 (define_predicate "aarch64_mem_pair_operand"
   (and (match_code "mem")
-       (match_test "aarch64_legitimate_address_p (mode, XEXP (op, 0), PARALLEL,
-					       0)")))
+       (match_test "aarch64_legitimate_address_p (mode, XEXP (op, 0),
+						  ADDR_QUERY_LDP_STP, 0)")))
 
 (define_predicate "aarch64_prefetch_operand"
   (match_test "aarch64_address_valid_for_prefetch_p (op, false)"))