From: Matthew Fortune <Matthew.Fortune@imgtec.com>
To: Richard Sandiford <rdsandiford@googlemail.com>,
Vladimir Makarov <vmakarov@redhat.com>
Cc: Robert Suchanek <Robert.Suchanek@imgtec.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
Kyrill Tkachov <kyrylo.tkachov@arm.com>
Subject: RE: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend
Date: Fri, 09 May 2014 22:58:00 -0000 [thread overview]
Message-ID: <6D39441BF12EF246A7ABCE6654B0235352881A@LEMAIL01.le.imgtec.org> (raw)
In-Reply-To: <87mweuww37.fsf@talisman.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
next prev parent reply other threads:[~2014-05-09 22:58 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-29 1:27 Robert Suchanek
2014-03-29 11:24 ` Richard Sandiford
2014-04-09 10:00 ` Robert Suchanek
2014-04-09 21:24 ` Richard Sandiford
2014-04-10 20:29 ` Richard Sandiford
2014-04-14 11:13 ` Robert Suchanek
2014-04-15 21:12 ` Richard Sandiford
2014-04-16 21:10 ` Robert Suchanek
2014-04-21 13:01 ` Richard Sandiford
2014-04-23 13:34 ` Robert Suchanek
2014-04-23 14:10 ` Richard Sandiford
2014-04-23 15:34 ` Robert Suchanek
2014-04-23 15:33 ` Vladimir Makarov
2014-05-03 19:21 ` Richard Sandiford
2014-05-06 14:06 ` Kyrill Tkachov
2014-05-06 19:22 ` Richard Sandiford
2014-05-09 22:58 ` Matthew Fortune [this message]
2014-05-10 18:44 ` Richard Sandiford
2014-05-14 13:24 ` Robert Suchanek
2014-05-15 21:05 ` Robert Suchanek
2014-05-15 21:34 ` Richard Sandiford
2014-05-16 21:05 ` Robert Suchanek
2014-05-18 19:38 ` Richard Sandiford
2014-06-02 19:37 ` RFA: Make LRA temporarily eliminate addresses before testing constraints Richard Sandiford
2014-06-03 20:50 ` Vladimir Makarov
2014-06-04 17:45 ` Richard Sandiford
2014-06-11 11:30 ` Robert Suchanek
2014-06-16 16:12 ` Robert Suchanek
2014-06-16 18:08 ` Vladimir Makarov
2014-06-18 20:52 ` Matthew Fortune
2014-03-29 14:08 ` [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend Jakub Jelinek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6D39441BF12EF246A7ABCE6654B0235352881A@LEMAIL01.le.imgtec.org \
--to=matthew.fortune@imgtec.com \
--cc=Robert.Suchanek@imgtec.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=kyrylo.tkachov@arm.com \
--cc=rdsandiford@googlemail.com \
--cc=vmakarov@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).