This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][rtlanal.c][BE][1/2] Fix vector load/stores to not use ld1/st1
- From: Richard Sandiford <richard dot sandiford at arm dot com>
- To: Alan Hayward <alan dot hayward at arm dot com>
- Cc: "gcc-patches\ at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, <ebotcazou at libertysurf dot fr>, <steven at gcc dot gnu dot org>
- Date: Fri, 12 Dec 2014 16:20:57 +0000
- Subject: Re: [PATCH][rtlanal.c][BE][1/2] Fix vector load/stores to not use ld1/st1
- Authentication-results: sourceware.org; auth=none
- References: <D0B07D0D dot 4108%alan dot hayward at arm dot com>
Alan Hayward <alan.hayward@arm.com> writes:
> [Cleaning this thread up to submit patch again, with better explanation]
>
> This patch causes subreg_get_info() to exit early in the simple cases
> where we are extracting a whole register from a multi register.
>
> In aarch64 for Big Endian we were producing a subreg of a OImode (256bits)
> from a CImode (384bits) This would hit the following assert in
> subreg_get_info:
>
> gcc_assert ((GET_MODE_SIZE (xmode) % GET_MODE_SIZE (ymode)) == 0);
>
>
> This is a rule we should be able to relax a little - if the subreg we want
> fits into a whole register then this is a valid result and can be easily
> detected earlier in the function.
>
> This has the bonus that we should be slightly reducing the execution time
> for more common cases, for example a subreg of 64bits from 256bits.
>
> This patch is required for the second part of the patch, which is aarch64
> specific, and fixes up aarch64 Big Endian movoi/ci/xi. This second part
> has already been approved.
>
> This patch will apply cleanly by itself and no regressions were seen when
> testing aarch64 and x86_64 on make check.
FWIW I agree this is the right approach, although I can't approve it.
The assert above is guarding code that deals with a very general case,
including some unusual combinations, so I don't think it would be a
good idea to try to remove it entirely.
E.g. Tejas hit the same assert because we were trying to create subregs
of EImode SIMD registers on AArch64. EImode is 24 bytes, so it's
one-*and-a-half* SIMD registers. Taking subregs of something like that is
very dangerous and I think we want the assert to continue to trigger there.
This patch deals with a much simpler and more obvious case.
Thanks,
Richard