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. >>> >>> 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? > > 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. > > Thanks, > James > >> >> 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. >
