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: Matthew Fortune <Matthew dot Fortune at imgtec dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>, Vladimir Makarov <vmakarov at redhat dot com>
- Cc: Robert Suchanek <Robert dot Suchanek at imgtec 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: Fri, 9 May 2014 22:58:32 +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>
Richard/Vlad,
Richard Sandiford <rdsandiford@googlemail.com> writes:
> Kyrill Tkachov <kyrylo.tkachov@arm.com> writes:
> > On 03/05/14 20:21, Richard Sandiford wrote:
...snip...
> > 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).
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.
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?
The patch below on top of Richard's addresses both problems and for the
fldmdbd test gets the correct output. I haven't done any more testing
than that though. I suspect there may be a better approach to achieve
the same effect but at least this shows what is wrong with the original
change.
Regards,
Matthew
diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index f59bf55..22bb273 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -348,6 +348,9 @@ valid_address_p (struct address_info *ad, const char *constraint = 0)
rtx saved_index_reg = NULL_RTX;
rtx *base_term = strip_subreg (ad->base_term);
rtx *index_term = strip_subreg (ad->index_term);
+#ifdef EXTRA_CONSTRAINT_STR
+ rtx orig_op = NULL_RTX;
+#endif
if (base_term != NULL)
{
saved_base_reg = *base_term;
@@ -360,9 +363,18 @@ valid_address_p (struct address_info *ad, const char *constraint = 0)
saved_index_reg = *index_term;
lra_eliminate_reg_if_possible (index_term);
}
+#ifdef EXTRA_CONSTRAINT_STR
+ if (ad->addr_outer_code == MEM)
+ {
+ orig_op = gen_rtx_MEM (ad->mode, *ad->outer);
+ MEM_ADDR_SPACE (orig_op) = ad->as;
+ }
+ else
+ orig_op = *ad->outer;
+#endif
bool ok_p = (constraint
#ifdef EXTRA_CONSTRAINT_STR
- ? EXTRA_CONSTRAINT_STR (*ad->outer, constraint[0], constraint)
+ ? EXTRA_CONSTRAINT_STR (orig_op, constraint[0], constraint)
#else
? false
#endif
@@ -2865,7 +2877,8 @@ process_address (int nop, rtx *before, rtx *after)
/* Target hooks sometimes reject extra constraint addresses -- use
EXTRA_CONSTRAINT_STR for the validation. */
if (constraint[0] != 'p'
- && EXTRA_ADDRESS_CONSTRAINT (constraint[0], constraint)
+ && (EXTRA_ADDRESS_CONSTRAINT (constraint[0], constraint)
+ || EXTRA_MEMORY_CONSTRAINT (constraint[0], constraint))
&& valid_address_p (&ad, constraint))
return change_p;
#endif