This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [patch] [MIPS] Implement Errata for 24K and 24KE
- From: Catherine Moore <clm at codesourcery dot com>
- To: Catherine Moore <clm at codesourcery dot com>, binutils at sourceware dot org, rdsandiford at googlemail dot com
- Date: Wed, 06 May 2009 18:48:25 -0400
- Subject: Re: [patch] [MIPS] Implement Errata for 24K and 24KE
- References: <49DD1319.7070108@codesourcery.com> <871vs1hr0f.fsf@firetop.home>
Hi Richard,
Here's a new patch which removes the warnings for .noreorder blocks and moves the check for the 24k
errata to isns_between. Does this patch look okay?
Catherine
2009-05-06 Catherine Moore <clm@codesourcery.com>
* config/tc-mips.c (check_for_24k_errata): Return the number
nops required instead of emitting them. Remove warnings for
noreorder sections.
(insns_between): Call check_for_24k_errata.
(md_mips_end): Remove call to check_for_24k_errata.
(append_insn): Likewise.
(start_noreorder): Likewise.
(s_change_sec): Likewise.
* testsuite/gas/mips/eret.l: Remove.
Index: config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.406
diff -p -r1.406 tc-mips.c
*** config/tc-mips.c 22 Apr 2009 11:40:25 -0000 1.406
--- config/tc-mips.c 6 May 2009 22:34:01 -0000
*************** reg_lookup (char **s, unsigned int types
*** 1797,1876 ****
/* Implement the ERET/DERET Errata for MIPS 24k.
- If an ERET/DERET is encountered in a noreorder block,
- warn if the ERET/DERET is followed by a branch instruction.
- Also warn if the ERET/DERET is the last instruction in the
- noreorder block.
-
IF an ERET/DERET is in a reorder block and is followed by a
branch instruction, insert a nop. */
! static void
! check_for_24k_errata (struct mips_cl_insn *insn, int eret_ndx)
! {
! bfd_boolean next_insn_is_branch = FALSE;
!
! /* eret_ndx will be -1 for the last instruction in a section
! and the ERET/DERET will be in insn, not history. */
! if (insn
! && eret_ndx == -1
! && (insn->insn_opcode == INSN_ERET
! || insn->insn_opcode == INSN_DERET)
! && insn->noreorder_p)
! {
! as_warn (_("ERET and DERET must be followed by a NOP on the 24K."));
! return;
! }
!
! if (history[eret_ndx].insn_opcode != INSN_ERET
! && history[eret_ndx].insn_opcode != INSN_DERET)
! return;
!
! if (!insn)
! {
! if (history[eret_ndx].noreorder_p)
! as_warn (_("ERET and DERET must be followed by a NOP on the 24K."));
! return;
! }
!
! next_insn_is_branch = ((insn->insn_opcode == INSN_ERET)
! || (insn->insn_opcode == INSN_DERET)
! || (insn->insn_mo->pinfo
! & (INSN_UNCOND_BRANCH_DELAY
! | INSN_COND_BRANCH_DELAY
! | INSN_COND_BRANCH_LIKELY)));
!
! if (next_insn_is_branch && history[eret_ndx].noreorder_p)
! {
! as_warn (_("ERET and DERET must be followed by a NOP on the 24K."));
! return;
! }
!
! /* Emit nop if the next instruction is a branch. */
! if (next_insn_is_branch)
! {
! long nop_where, br_where;
! struct frag *nop_frag, *br_frag;
! struct mips_cl_insn br_insn, nop_insn;
!
! emit_nop ();
! nop_insn = history[eret_ndx - 1];
! nop_frag = history[eret_ndx - 1].frag;
! nop_where = history[eret_ndx - 1].where;
! br_insn = history[eret_ndx];
! br_frag = history[eret_ndx].frag;
! br_where = history[eret_ndx].where;
! move_insn (&nop_insn, br_frag, br_where);
! move_insn (&br_insn, nop_frag, nop_where);
! history[eret_ndx-1] = br_insn;
! history[eret_ndx] = nop_insn;
! }
}
/* Return TRUE if opcode MO is valid on the currently selected ISA and
architecture. If EXPANSIONP is TRUE then this check is done while
expanding a macro. Use is_opcode_valid_16 for MIPS16 opcodes. */
--- 1797,1830 ----
/* Implement the ERET/DERET Errata for MIPS 24k.
IF an ERET/DERET is in a reorder block and is followed by a
branch instruction, insert a nop. */
! static int
! check_for_24k_errata (const struct mips_cl_insn *insn1,
! const struct mips_cl_insn *insn2)
! {
! if (insn1->noreorder_p
! || (insn1->insn_opcode != INSN_ERET
! && insn1->insn_opcode != INSN_DERET))
! return 0;
! if (insn1->noreorder_p)
! return 0;
! if ((insn2->insn_opcode == INSN_ERET)
! || (insn2->insn_opcode == INSN_DERET)
! || (insn2->insn_mo->pinfo
! & (INSN_UNCOND_BRANCH_DELAY
! | INSN_COND_BRANCH_DELAY
! | INSN_COND_BRANCH_LIKELY)))
! return 1;
! return 0;
}
/* Return TRUE if opcode MO is valid on the currently selected ISA and
architecture. If EXPANSIONP is TRUE then this check is done while
expanding a macro. Use is_opcode_valid_16 for MIPS16 opcodes. */
*************** md_begin (void)
*** 2156,2164 ****
void
md_mips_end (void)
{
- if (mips_fix_24k)
- check_for_24k_errata ((struct mips_cl_insn *) &history[0], -1);
-
if (! ECOFF_DEBUGGING)
md_obj_end ();
}
--- 2110,2115 ----
*************** insns_between (const struct mips_cl_insn
*** 2573,2578 ****
--- 2524,2532 ----
&& INSN2_USES_REG (EXTRACT_OPERAND (RD, *insn1), MIPS_GR_REG))
return 2;
+ if (mips_fix_24k)
+ return check_for_24k_errata (insn1, insn2);
+
/* If working around VR4120 errata, check for combinations that need
a single intervening instruction. */
if (mips_fix_vr4120)
*************** append_insn (struct mips_cl_insn *ip, ex
*** 3390,3400 ****
insn information. */
if (pinfo & INSN_UNCOND_BRANCH_DELAY)
{
- /* Check for eret/deret before clearing history. */
- if (mips_fix_24k)
- check_for_24k_errata (
- (struct mips_cl_insn *) &history[hndx_24k],
- hndx_24k+1);
mips_no_prev_insn ();
}
}
--- 3344,3349 ----
*************** append_insn (struct mips_cl_insn *ip, ex
*** 3415,3424 ****
else
insert_into_history (0, 1, ip);
- if (mips_fix_24k)
- check_for_24k_errata ((struct mips_cl_insn *) &history[hndx_24k],
- hndx_24k+1);
-
/* We just output an insn, so the next one doesn't have a label. */
mips_clear_insn_labels ();
}
--- 3364,3369 ----
*************** start_noreorder (void)
*** 3505,3512 ****
static void
end_noreorder (void)
{
- if (mips_fix_24k)
- check_for_24k_errata (NULL, 0);
mips_opts.noreorder--;
if (mips_opts.noreorder == 0 && prev_nop_frag != NULL)
--- 3450,3455 ----
*************** s_change_sec (int sec)
*** 12589,12597 ****
mips_emit_delays ();
- if (mips_fix_24k)
- check_for_24k_errata ((struct mips_cl_insn *) &history[0], -1);
-
switch (sec)
{
case 't':
--- 12532,12537 ----
*************** s_change_section (int ignore ATTRIBUTE_U
*** 12650,12658 ****
if (!IS_ELF)
return;
- if (mips_fix_24k)
- check_for_24k_errata ((struct mips_cl_insn *) &history[0], -1);
-
section_name = input_line_pointer;
c = get_symbol_end ();
if (c)
--- 12590,12595 ----
Richard Sandiford wrote:
Catherine Moore <clm@codesourcery.com> writes:
This patch implements the errata for the 24K and 24KE. The errata
calls for a NOP to be emitted after an ERET/DERET instruction followed
by a branch. If the ERET/DERET is in a noreorder section, then a
warning will be issued instead. This functionality requires the
-mfix-24k option.
In addition, there was an error in the delay slot handling in
append_insn. The code failed to exclude an ERET or DERET instuction
from being moved into a delay slot. That failure is also corrected
with this patch.
Does this look okay to install?
Other hazards of this kind are handled by insns_between. Was there
a reason why you couldn't use it in this case?
I don't like the idea of warning about an ERET at the end of a
.set noreorder block. In general, if you have:
.set noreorder
...
FOO
...
.set reorder
...
BAR
and there is a hazard between FOO and BAR, it is perfectly OK to insert
nops at any point between the ".set reorder" and BAR. This is what we
do for other hazards, and I think we should be consistent. Likewise
if you have:
FOO
...
.set noreorder
...
BAR
...
.set reorder
it is perfectly OK to insert nops between FOO and the ".set noreorder".
(FWIW, using insns_between should give you this for free.)
Also, ".set neorder" has traditionally meant "trust me!". If we want
to warn about caess where we think the programmer is wrong, I think
we need a consistent story.
Richard