This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Matthew Fortune <Matthew dot Fortune at imgtec dot com>
- Cc: Andrew Bennett <Andrew dot Bennett at imgtec dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, "Moore, Catherine (Catherine_Moore at mentor dot com)" <Catherine_Moore at mentor dot com>
- Date: Wed, 19 Nov 2014 00:53:35 +0000
- Subject: RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop
- Authentication-results: sourceware.org; auth=none
- References: <0DA23CC379F5F945ACB41CF394B9827720F331A8 at LEMAIL01 dot le dot imgtec dot org> <alpine dot DEB dot 1 dot 10 dot 1411181345380 dot 2881 at tp dot orcam dot me dot uk> <0DA23CC379F5F945ACB41CF394B9827720F332BC at LEMAIL01 dot le dot imgtec dot org> <alpine dot DEB dot 1 dot 10 dot 1411181450250 dot 2881 at tp dot orcam dot me dot uk> <0DA23CC379F5F945ACB41CF394B9827720F333C0 at LEMAIL01 dot le dot imgtec dot org> <alpine dot DEB dot 1 dot 10 dot 1411181551000 dot 2881 at tp dot orcam dot me dot uk> <0DA23CC379F5F945ACB41CF394B9827720F3348E at LEMAIL01 dot le dot imgtec dot org> <6D39441BF12EF246A7ABCE6654B0235320F73EA2 at LEMAIL01 dot le dot imgtec dot org>
On Tue, 18 Nov 2014, Matthew Fortune wrote:
> > > With a quick look at `mips_process_sync_loop' it looks to me the
> > > other NOP is produced from here:
> > >
> > > else if (!(required_oldval && cmp))
> > > mips_multi_add_insn ("nop", NULL);
> > >
> > > -- correct? If so, then can't you just make it:
> >
> > Correct.
> >
> > >
> > > else if (!(required_oldval && cmp) && !TARGET_FIX_R10000)
> > > mips_multi_add_insn ("nop", NULL);
> > >
> > > instead? It appears plain and simple, and if you're concerned about
> > > it being unobvious to the casual reader, then add a one-line comment
> > > or suchlike. It's not like TARGET_FIX_R10000 is going to imply
> > > anything else about branches in the future (and r6 code won't run on
> > > an R10k anyway).
>
> I'm afraid I disagree about it being plain and simple even with a comment.
> The amount of code in the backend that makes assumptions based on derived
> variables is very high and it makes the code hard to read (especially w.r.t.
> branches). I'm OK with adding the code to avoid the extra NOP but have it
> based on mips_branch_likely and fix the callers so that it is set
> appropriately.
Sure, fine with me too. It looks to me it'll be more natural even to
have `mips_branch_likely' preset on entering `mips_process_sync_loop'.
Maciej