[patch] [MIPS] Implement Errata for 24K and 24KE

Catherine Moore clm@codesourcery.com
Wed May 6 22:49:00 GMT 2009


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



More information about the Binutils mailing list