On 18/06/18 09:08, Andre Simoes Dias Vieira wrote:
> Hi Richard,
>
> Sorry for the delay I have been on holidays. I had a look and I think you
> are right. With these changes Umq and Uml seem to have the same
> functionality though, so I would suggest using only one. Maybe use a
> different name for both, removing both Umq and Uml in favour of Umn, where
> the n indicates it narrows the addressing mode. How does that sound to you?
>
> I also had a look at Ump, but that one is used in the parallel pattern for
> STP/LDP which does not use this "narrowing". So we should leave that one as
> is.
>
> Cheers,
> Andre
>
> ________________________________________
> From: Richard Sandiford <richard.sandif...@arm.com>
> Sent: Thursday, June 14, 2018 12:28:16 PM
> To: Andre Simoes Dias Vieira
> Cc: gcc-patches@gcc.gnu.org; nd
> Subject: Re: [AArch64][PATCH 1/2] Fix addressing printing of LDP/STP
>
> Andre Simoes Dias Vieira <andre.simoesdiasvie...@arm.com> writes:
>> @@ -5716,10 +5717,17 @@ aarch64_classify_address (struct
>> aarch64_address_info *info,
>> unsigned int vec_flags = aarch64_classify_vector_mode (mode);
>> bool advsimd_struct_p = (vec_flags == (VEC_ADVSIMD | VEC_STRUCT));
>> bool load_store_pair_p = (type == ADDR_QUERY_LDP_STP
>> + || type == ADDR_QUERY_LDP_STP_N
>> || mode == TImode
>> || mode == TFmode
>> || (BYTES_BIG_ENDIAN && advsimd_struct_p));
>>
>> + /* If we are dealing with ADDR_QUERY_LDP_STP_N that means the incoming
>> mode
>> + corresponds to the actual size of the memory being loaded/stored and
>> the
>> + mode of the corresponding addressing mode is half of that. */
>> + if (type == ADDR_QUERY_LDP_STP_N && known_eq (GET_MODE_SIZE (mode), 16))
>> + mode = DFmode;
>> +
>> bool allow_reg_index_p = (!load_store_pair_p
>> && (known_lt (GET_MODE_SIZE (mode), 16)
>> || vec_flags == VEC_ADVSIMD
>
> I don't know whether it matters in practice, but that description also
> applies to Umq, not just Uml. It might be worth changing it too so
> that things stay consistent.
>
> Thanks,
> Richard
>
Hi all,
This is a reworked patched, replacing Umq and Uml with Umn now.
Bootstrapped and tested on aarch64-none-linux-gnu.
Is this OK for trunk?
gcc
2018-06-25 Andre Vieira <andre.simoesdiasvie...@arm.com>
* config/aarch64/aarch64-simd.md (aarch64_simd_mov<VQ:mode>):
Replace
Umq with Umn.
(store_pair_lanes<mode>): Likewise.
* config/aarch64/aarch64-protos.h (aarch64_addr_query_type): Add new
enum value 'ADDR_QUERY_LDP_STP_N'.
* config/aarch64/aarch64.c (aarch64_addr_query_type): Likewise.
(aarch64_print_address_internal): Add declaration.
(aarch64_print_ldpstp_address): Remove.
(aarch64_classify_address): Adapt mode for 'ADDR_QUERY_LDP_STP_N'.
(aarch64_print_operand): Change printing of 'y'.
* config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand): Use
new enum value 'ADDR_QUERY_LDP_STP_N', don't hardcode mode and use
'true' rather than '1'.
* gcc/config/aarch64/constraints.md (Uml): Likewise.
(Uml): Rename to Umn.
(Umq): Remove.
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 4ea50acaa59c0b58a213bd1f27fb78b6d8deee96..c03a442107815eed44a3b6bceb386d78a6615483 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -120,6 +120,10 @@ enum aarch64_symbol_type
ADDR_QUERY_LDP_STP
Query what is valid for a load/store pair.
+ ADDR_QUERY_LDP_STP_N
+ Query what is valid for a load/store pair, but narrow the incoming mode
+ for address checking. This is used for the store_pair_lanes patterns.
+
ADDR_QUERY_ANY
Query what is valid for at least one memory constraint, which may
allow things that "m" doesn't. For example, the SVE LDR and STR
@@ -128,6 +132,7 @@ enum aarch64_symbol_type
enum aarch64_addr_query_type {
ADDR_QUERY_M,
ADDR_QUERY_LDP_STP,
+ ADDR_QUERY_LDP_STP_N,
ADDR_QUERY_ANY
};
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 4e5c42b0f15b863f3088dba4aac450f31ca309bb..7936581947b360a6d6a88ce6523bbb804c3eb89c 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -131,7 +131,7 @@
(define_insn "*aarch64_simd_mov<VQ:mode>"
[(set (match_operand:VQ 0 "nonimmediate_operand"
- "=w, Umq, m, w, ?r, ?w, ?r, w")
+ "=w, Umn, m, w, ?r, ?w, ?r, w")
(match_operand:VQ 1 "general_operand"
"m, Dz, w, w, w, r, r, Dn"))]
"TARGET_SIMD
@@ -3059,7 +3059,7 @@
)
(define_insn "store_pair_lanes<mode>"
- [(set (match_operand:<VDBL> 0 "aarch64_mem_pair_lanes_operand" "=Uml, Uml")
+ [(set (match_operand:<VDBL> 0 "aarch64_mem_pair_lanes_operand" "=Umn, Umn")
(vec_concat:<VDBL>
(match_operand:VDC 1 "register_operand" "w, r")
(match_operand:VDC 2 "register_operand" "w, r")))]
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index c94f7093b0880aee17a4f9b7a3a60b2c27a62f98..1e702248c51c61ac881c1a47a3a509f718823de1 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -206,7 +206,8 @@ static bool aarch64_builtin_support_vector_misalignment (machine_mode mode,
int misalignment,
bool is_packed);
static machine_mode aarch64_simd_container_mode (scalar_mode, poly_int64);
-static bool aarch64_print_ldpstp_address (FILE *, machine_mode, rtx);
+static bool aarch64_print_address_internal (FILE*, machine_mode, rtx,
+ aarch64_addr_query_type);
/* Major revision number of the ARM Architecture implemented by the target. */
unsigned aarch64_architecture_version;
@@ -5716,10 +5717,18 @@ aarch64_classify_address (struct aarch64_address_info *info,
unsigned int vec_flags = aarch64_classify_vector_mode (mode);
bool advsimd_struct_p = (vec_flags == (VEC_ADVSIMD | VEC_STRUCT));
bool load_store_pair_p = (type == ADDR_QUERY_LDP_STP
+ || type == ADDR_QUERY_LDP_STP_N
|| mode == TImode
|| mode == TFmode
|| (BYTES_BIG_ENDIAN && advsimd_struct_p));
+ /* If we are dealing with ADDR_QUERY_LDP_STP_N that means the incoming mode
+ corresponds to the actual size of the memory being loaded/stored and the
+ mode of the corresponding addressing mode is half of that. */
+ if (type == ADDR_QUERY_LDP_STP_N
+ && known_eq (GET_MODE_SIZE (mode), 16))
+ mode = DFmode;
+
bool allow_reg_index_p = (!load_store_pair_p
&& (known_lt (GET_MODE_SIZE (mode), 16)
|| vec_flags == VEC_ADVSIMD
@@ -7094,13 +7103,10 @@ aarch64_print_operand (FILE *f, rtx x, int code)
return;
}
- if (code == 'y')
- /* LDP/STP which uses a single double-width memory operand.
- Adjust the mode to appear like a typical LDP/STP.
- Currently this is supported for 16-byte accesses only. */
- mode = DFmode;
-
- if (!aarch64_print_ldpstp_address (f, mode, XEXP (x, 0)))
+ if (!aarch64_print_address_internal (f, mode, XEXP (x, 0),
+ code == 'y'
+ ? ADDR_QUERY_LDP_STP_N
+ : ADDR_QUERY_LDP_STP))
output_operand_lossage ("invalid operand prefix '%%%c'", code);
}
break;
@@ -7223,13 +7229,6 @@ aarch64_print_address_internal (FILE *f, machine_mode mode, rtx x,
return false;
}
-/* Print address 'x' of a LDP/STP with mode 'mode'. */
-static bool
-aarch64_print_ldpstp_address (FILE *f, machine_mode mode, rtx x)
-{
- 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)
diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md
index 32a0fa60a198c714f7c0b8b987da6bc26992845d..72cacdabdac52dcb40b480f7a5bfbf4997c742d8 100644
--- a/gcc/config/aarch64/constraints.md
+++ b/gcc/config/aarch64/constraints.md
@@ -218,14 +218,6 @@
(and (match_code "mem")
(match_test "REG_P (XEXP (op, 0))")))
-(define_memory_constraint "Umq"
- "@internal
- 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), false,
- ADDR_QUERY_LDP_STP)")))
-
(define_memory_constraint "Ump"
"@internal
A memory address suitable for a load/store pair operation."
@@ -233,14 +225,16 @@
(match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP (op, 0),
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.
-(define_memory_constraint "Uml"
+;; Used for storing or loading pairs in an AdvSIMD register using an STP/LDP
+;; as a vector-concat. The address mode uses the same constraints as if it
+;; were for a single value.
+(define_memory_constraint "Umn"
"@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), 1,
- ADDR_QUERY_LDP_STP)")))
+ (match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP (op, 0),
+ true,
+ ADDR_QUERY_LDP_STP_N)")))
(define_memory_constraint "Utr"
"@internal
diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
index 7aec76d681f5eca87b7b5e1d63d12dc0205ad113..f0917af8b4cec945ba4e38e4dc670200f8812983 100644
--- a/gcc/config/aarch64/predicates.md
+++ b/gcc/config/aarch64/predicates.md
@@ -226,8 +226,9 @@
;; 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), 1,
- ADDR_QUERY_LDP_STP)")))
+ (match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP (op, 0),
+ true,
+ ADDR_QUERY_LDP_STP_N)")))
(define_predicate "aarch64_prefetch_operand"
(match_test "aarch64_address_valid_for_prefetch_p (op, false)"))
