This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][AArch64] Improve aarch64_legitimate_constant_p
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>
- Cc: Richard Sandiford <richard dot sandiford at linaro dot org>, Andreas Schwab <schwab at linux-m68k dot org>, James Greenhalgh <James dot Greenhalgh at arm dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>
- Date: Wed, 10 Jan 2018 14:55:19 +0100
- Subject: Re: [PATCH][AArch64] Improve aarch64_legitimate_constant_p
- Authentication-results: sourceware.org; auth=none
- References: <AM5PR0802MB2610272348FB1C5DADA9665C83AA0@AM5PR0802MB2610.eurprd08.prod.outlook.com> <20171026153853.GC25439@arm.com> <DB6PR0801MB2053860EE7894E7B973F1AB1835E0@DB6PR0801MB2053.eurprd08.prod.outlook.com> <87vaiqkl1j.fsf@linux-m68k.org> <87efpby92p.fsf@linaro.org> <HE1PR0801MB2058398C14D58CA2D815BEFB83500@HE1PR0801MB2058.eurprd08.prod.outlook.com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Mon, Nov 06, 2017 at 01:50:07PM +0000, Wilco Dijkstra wrote:
> Richard Sandiford wrote:
> >
> > Yeah, I'd hit this too. I think it's a latent bug that just
> > happened to be exposed by Wilco's patch: although the *movti_aarch64
> > predicate disallows const_wide_int, the constraints allow it via "n",
> > which means that the RA can rematerialise a const_wide_int that would
> > otherwise be spilled or forced to memory.
>
> Yes I explicitly disallowed const-wide-int because otherwise it failed in
> Fortran code. Clearly there were more corner cases...
>
> > Maybe the best fix would be just to go ahead and add support for
> > const_wide_int, as with the patch below.
>
> But then it always uses a MOV/MOVK expansion, no matter how complex.
> That's inefficient since it would take at most 8 instructions. It's best to load
> complex immediates from the literal pool, so we need a cutoff (eg. sum of
> aarch64_internal_mov_immediate of both halves <= 4), and use a literal load
> otherwise, just like we do for floating point constants.
Can we apply Richard's patch and then perhaps incrementally improve stuff,
like selecting a subset of CONST_WIDE_INTs we want to accept for movti/movtf
and corresponding constraint that would be used instead of the "n" in
*movti_aarch64?
I mean, the trunk shouldn't remain so broken for months after a correct
patch has been posted.
For the new predicate/constraint, you can e.g. have a look at i386
x86_64_hilo_int_operand predicate which does a similar thing and is used for
similar stuff, namely a constant that is meant to be split and each half
needs to satisfy something. If it is a purely performance/-Os thing, you
could as well count the number of instructions you'll need to construct it
or similar. E.g. in the pr83726.C case, even when it is a CONST_WIDE_INT,
it is a rather cheap one, one half is 70, the other half 0.
Jakub