| Message ID | 8760dgc5wc.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
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.
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 <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)"))
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)"))
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. > >
James Greenhalgh <james.greenhalgh@arm.com> writes: > 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> Thanks. I needed to update it slightly after recent changes to config/aarch64, but the differences from the first one were obvious. here's what I applied. (And I just realised I forgot the Reviewed-By:, sorry about that.) Richard 2017-12-21 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_print_address_internal): Take an aarch64_addr_query_type instead of an rtx code. (aarch64_address_valid_for_prefetch_p): Update calls accordingly. (aarch64_legitimate_address_hook_p): Likewise. (aarch64_print_ldpstp_address): Likewise. (aarch64_print_operand_address): Likewise. (aarch64_address_cost): Likewise. * config/aarch64/constraints.md (Uml, Umq, Ump, Utq): Likewise. * config/aarch64/predicates.md (aarch64_mem_pair_operand): Likewise. (aarch64_mem_pair_lanes_operand): Likewise. Index: gcc/config/aarch64/aarch64-protos.h =================================================================== --- gcc/config/aarch64/aarch64-protos.h 2017-12-15 13:09:53.224711621 +0000 +++ gcc/config/aarch64/aarch64-protos.h 2017-12-20 23:03:10.548917491 +0000 @@ -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. */ @@ -440,7 +453,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-12-16 14:10:35.780523658 +0000 +++ gcc/config/aarch64/aarch64.c 2017-12-20 23:03:10.549917461 +0000 @@ -4437,21 +4437,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 @@ -4683,7 +4683,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; @@ -4719,19 +4719,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. @@ -5630,14 +5629,15 @@ #define buf_size 20 'op' is the context required by aarch64_classify_address. It can either be MEM for a normal memory access or PARALLEL for LDP/STP. */ static bool -aarch64_print_address_internal (FILE *f, machine_mode mode, rtx x, RTX_CODE op) +aarch64_print_address_internal (FILE *f, machine_mode mode, rtx x, + aarch64_addr_query_type type) { struct aarch64_address_info addr; /* Check all addresses are Pmode - including ILP32. */ gcc_assert (GET_MODE (x) == Pmode); - if (aarch64_classify_address (&addr, x, mode, op, true)) + if (aarch64_classify_address (&addr, x, mode, true, type)) switch (addr.type) { case ADDRESS_REG_IMM: @@ -5725,14 +5725,14 @@ aarch64_print_address_internal (FILE *f, static bool aarch64_print_ldpstp_address (FILE *f, machine_mode mode, rtx x) { - return aarch64_print_address_internal (f, mode, x, PARALLEL); + return aarch64_print_address_internal (f, mode, x, ADDR_QUERY_LDP_STP); } /* Print address 'x' of a memory access with mode 'mode'. */ static void aarch64_print_operand_address (FILE *f, machine_mode mode, rtx x) { - if (!aarch64_print_address_internal (f, mode, x, MEM)) + if (!aarch64_print_address_internal (f, mode, x, ADDR_QUERY_M)) output_addr_const (f, x); } @@ -6561,7 +6561,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-11-29 11:06:26.801567667 +0000 +++ gcc/config/aarch64/constraints.md 2017-12-20 23:03:10.549917461 +0000 @@ -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)"))) ;; Used for storing two 64-bit values in an AdvSIMD register using an STP ;; as a 128-bit vec_concat. @@ -177,8 +177,8 @@ (define_memory_constraint "Uml" "@internal A memory address suitable for a load/store pair operation." (and (match_code "mem") - (match_test "aarch64_legitimate_address_p (DFmode, XEXP (op, 0), - PARALLEL, 1)"))) + (match_test "aarch64_legitimate_address_p (DFmode, XEXP (op, 0), 1, + ADDR_QUERY_LDP_STP)"))) (define_memory_constraint "Utv" "@internal @@ -191,8 +191,8 @@ (define_memory_constraint "Utq" "@internal An address valid for loading or storing a 128-bit AdvSIMD register" (and (match_code "mem") - (match_test "aarch64_legitimate_address_p (V2DImode, XEXP (op, 0), - MEM, 1)"))) + (match_test "aarch64_legitimate_address_p (V2DImode, + XEXP (op, 0), 1)"))) (define_constraint "Ufc" "A floating point constant which can be used with an\ Index: gcc/config/aarch64/predicates.md =================================================================== --- gcc/config/aarch64/predicates.md 2017-12-15 13:09:52.182101079 +0000 +++ gcc/config/aarch64/predicates.md 2017-12-20 23:03:10.550917431 +0000 @@ -186,15 +186,15 @@ (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)"))) ;; Used for storing two 64-bit values in an AdvSIMD register using an STP ;; as a 128-bit vec_concat. (define_predicate "aarch64_mem_pair_lanes_operand" (and (match_code "mem") - (match_test "aarch64_legitimate_address_p (DFmode, XEXP (op, 0), - PARALLEL, 1)"))) + (match_test "aarch64_legitimate_address_p (DFmode, XEXP (op, 0), 1, + ADDR_QUERY_LDP_STP)"))) (define_predicate "aarch64_prefetch_operand" (match_test "aarch64_address_valid_for_prefetch_p (op, false)"))
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)"))