This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend
- From: Robert Suchanek <Robert dot Suchanek at imgtec dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>, Matthew Fortune <Matthew dot Fortune at imgtec dot com>
- Cc: Vladimir Makarov <vmakarov at redhat dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Kyrill Tkachov <kyrylo dot tkachov at arm dot com>
- Date: Thu, 15 May 2014 21:05:08 +0000
- Subject: RE: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend
- Authentication-results: sourceware.org; auth=none
- References: <B5E67142681B53468FAF6B7C313565623D3DD70C at KLMAIL01 dot kl dot imgtec dot org> <87d2h51dm6 dot fsf at talisman dot default> <B5E67142681B53468FAF6B7C313565623D3E3FED at KLMAIL01 dot kl dot imgtec dot org> <87d2gqfb7t dot fsf at talisman dot default> <B5E67142681B53468FAF6B7C313565623D3E4420 at KLMAIL01 dot kl dot imgtec dot org> <87ob02jodp dot fsf at talisman dot default> <B5E67142681B53468FAF6B7C313565623D3E460A at KLMAIL01 dot kl dot imgtec dot org> <87fvl6hnw2 dot fsf at talisman dot default> <5357D588 dot 6000202 at redhat dot com> <87tx967jnq dot fsf at talisman dot default> <5368EC5F dot 3010006 at arm dot com> <87mweuww37 dot fsf at talisman dot default> <6D39441BF12EF246A7ABCE6654B0235352881A at LEMAIL01 dot le dot imgtec dot org> <87vbtd1njm dot fsf at talisman dot default>,<B5E67142681B53468FAF6B7C313565623D3E73AF at KLMAIL01 dot kl dot imgtec dot org>
Ping.
________________________________________
From: Robert Suchanek
Sent: 14 May 2014 14:24
To: Richard Sandiford; Matthew Fortune
Cc: Vladimir Makarov; gcc-patches@gcc.gnu.org; Kyrill Tkachov
Subject: RE: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend
Hi Richard,
Are you working on the solution to fix the breakage? I'm about
to look into this and wanted to find out how far we got with this.
Regards,
Robert
> -----Original Message-----
> From: Richard Sandiford [mailto:rdsandiford@googlemail.com]
> Sent: 10 May 2014 19:44
> To: Matthew Fortune
> Cc: Vladimir Makarov; Robert Suchanek; gcc-patches@gcc.gnu.org; Kyrill
> Tkachov
> Subject: Re: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend
>
> Thanks for looking at this.
>
> Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> >> > Hi all,
> >> > This caused some testsuite failures on arm:
> >> > FAIL: gcc.target/arm/vfp-ldmdbd.c scan-assembler fldmdbd
> >> > FAIL: gcc.target/arm/vfp-ldmdbs.c scan-assembler fldmdbs
> >> > FAIL: gcc.target/arm/vfp-ldmiad.c scan-assembler fldmiad
> >> > FAIL: gcc.target/arm/vfp-ldmias.c scan-assembler fldmias
> >> >
> >> > From the vfp-ldmdbd.c test this patch changed the codegen from:
> >> > fldmdbd r5!, {d7}
> >> >
> >> > into
> >> > sub r5, r5, #8
> >> > fldd d7, [r5]
> >> >
> >> > Which broke the test.
> >>
> >> Sorry for the breakage. I've reverted the patch for now and will send a
> >> fixed version when I have time.
> >
> > The problem appears to lie with the new satisfies_memory_constraint_p
> > which is passing just the address to valid_address_p but the constraint
> > is a memory constraint (which wants the mem to be retained).
>
> Yeah.
>
> > One option is to re-construct the MEM using the address_info provided
> > to valid_address_p. This has mode, address space and whether it was
> > actually a MEM to start with so there is enough information.
>
> We don't want to create garbage rtl though. The substitution happens
> in-place, so the routine does temporarily turn the original MEM into the
> eliminated form.
>
> My first reaction after seeing the bug was to pass the operand in as
> another parameter to valid_address_p. That made the interface a bit
> ridiculous though, so I didn't post it and reverted the original patch
> instead.
>
> I think a cleaner way of doing it would be to have helper functions
> that switch in and out of the eliminated form, storing the old form
> in fields of a new structure (either separate from address_info,
> or a local inheritance of it). We probably also want to have arrays
> of address_infos, one for each operand, so that we don't analyse the
> same address too many times during the same insn.
>
> > Another issue noticed while looking at this:
> > process_address does not seem to make an attempt to check for
> > EXTRA_MEMORY_CONSTRAINT even though it does decompose memory addresses.
> > For the lea address case then the address is checked with valid_address_p.
> > This seems inconsistent, is it intentional?
>
> Yeah, I think so. Memory constraints are different in two main ways:
>
> (a) it's obvious from the MEM what the mode and address space of the
> access actually are. legitimate_address_p is all about the most
> general address, so in practice no memory constraint should rely
> on accepting more than legitimate_address_p does.
>
> (b) it's useful for one pattern to have several alternatives with
> different memory constraints (e.g. "ZS", "ZT" and "m" in the 32-bit
> MIPS move patterns). There isn't really anything special about the
> first alternative.
>
> IMO it'd be good to get rid of this special handling for extra address
> constraints, but I've no idea how easy that would be.
>
> Thanks,
> Richard