This is the mail archive of the
mailing list for the GCC project.
Re: [AArch64][PATCH 1/2] Fix addressing printing of LDP/STP
- From: "Andre Vieira (lists)" <Andre dot SimoesDiasVieira at arm dot com>
- To: James Greenhalgh <james dot greenhalgh at arm dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Richard Sandiford <Richard dot Sandiford at arm dot com>, Kyrylo Tkachov <Kyrylo dot Tkachov at arm dot com>
- Date: Thu, 19 Jul 2018 15:08:41 +0100
- Subject: Re: [AArch64][PATCH 1/2] Fix addressing printing of LDP/STP
- References: <AM6PR08MB3254F356249918948A2B17FB88640@AM6PR08MB3254.eurprd08.prod.outlook.com> <firstname.lastname@example.org> <AM6PR08MB3254872341D39F3D9D70E21188710@AM6PR08MB3254.eurprd08.prod.outlook.com> <5B30AC4D.email@example.com> <20180717145226.GA35695@arm.com>
On 17/07/18 15:52, James Greenhalgh wrote:
> On Mon, Jun 25, 2018 at 03:48:13AM -0500, Andre Simoes Dias Vieira wrote:
>> 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.
>>> From: Richard Sandiford <firstname.lastname@example.org>
>>> Sent: Thursday, June 14, 2018 12:28:16 PM
>>> To: Andre Simoes Dias Vieira
>>> Cc: email@example.com; nd
>>> Subject: Re: [AArch64][PATCH 1/2] Fix addressing printing of LDP/STP
>>> Andre Simoes Dias Vieira <Andre.SimoesDiasVieira@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.
>> 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?
> OK. Does this also need backporting to 8?
It doesn't __need__ to, as the failure this fixes has not been observed
on gcc-8, I assume that is because of our restrictive LDP/STP
generation (see patch 2/2 of this series). Though it wouldn't hurt
either I think.
>> 2018-06-25 Andre Vieira <firstname.lastname@example.org>
>> * config/aarch64/aarch64-simd.md (aarch64_simd_mov<VQ:mode>):
>> 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.