RE: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend

[Robert Suchanek <Robert.Suchanek@imgtec.com>]

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