This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]