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


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


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