This is the mail archive of the binutils@sourceware.org mailing list for the binutils 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: [PATCH] MIPS: microMIPS ASE support


This is the second and final part of the review.

First of all, thanks for the great effort you've made to integrate the
microMIPS macro code into macro().  This is much, much better than before,
and should be far less of a maintenance burden.

FWIW, I plan to review the follow-up patch based purely on whether
it deals with the review comments.  Anything that we've collectively
missed this far (and there's bound to be something) will just have to
be fixed when someone trips over it.

I think the only change of any significant size that needs to be made
is to move the place where we convert to microMIPS relocs (see below).
Everything else is pretty small.

> @@ -1821,6 +1830,8 @@ the target word.  These are used on the 
>  ENUM
>    BFD_RELOC_GPREL16
>  ENUMX
> +  BFD_RELOC_MICROMIPS_GPREL16
> +ENUMX
>    BFD_RELOC_GPREL32
>  ENUMDOC
>    For systems that allocate a Global Pointer register, these are

You've now moved most of the new relocs into MIPS-specific areas, thanks,
but some, like the one above, are still in generic lists.  Let's put:

ENUM
  BFD_RELOC_MICROMIPS_7_PCREL_S1
ENUMX
  BFD_RELOC_MICROMIPS_10_PCREL_S1
ENUMX
  BFD_RELOC_MICROMIPS_16_PCREL_S1
ENUMDOC
  MicroMIPS PC-relative relocations.

before the main "MIPS ELF relocations." section, followed
by a new section:

ENUM
  BFD_RELOC_MICROMIPS_GPREL16
ENUMX
  BFD_RELOC_MICROMIPS_HI16
ENUMX
  BFD_RELOC_MICROMIPS_HI16_S
ENUMX
  BFD_RELOC_MICROMIPS_LO16
ENUMDOC
  MicroMIPS versions of generic BFD relocs.

Also, let's combine this:

> @@ -2182,6 +2193,11 @@ ENUMDOC
>       simple reloc otherwise.
>  
>  ENUM
> +  BFD_RELOC_MICROMIPS_JMP
> +ENUMDOC
> +  The microMIPS jump instruction.
> +
> +ENUM
>    BFD_RELOC_MIPS16_JMP
>  ENUMDOC
>    The MIPS16 jump instruction.

with the MIPS_JMP entry, since you're doing the same with the other
microMIPS versions of MIPS relocs.

> +  /* Whether we are assembling for the mipsMIPS processor.  0 if we are
> +     not, 1 if we are, and -1 if the value has not been initialized.
> +     Changed by `.set micromips' and `.set nomicromips', and the -mmicromips
> +     and -mno-micromips command line options, and the default CPU.  */
> +  int micromips;

Blind cut-&-paste.  "microMIPS ASE".

Let's leave the -1 case and:

> +/* Return true if the given CPU supports microMIPS.  */
> +#define CPU_HAS_MICROMIPS(cpu)	0

out.  I think the CPU_HAS_MIPS16 stuff is derived from the original LSI
TinyRisc support and wouldn't be used for ASEs.

> @@ -495,9 +515,11 @@ static int mips_32bitmode = 0;
>     require nops to be inserted.  This applies to instructions marked
>     INSN_LOAD_MEMORY_DELAY.  These nops are only required at MIPS ISA
>     level I.  */
> -#define gpr_interlocks \
> -  (mips_opts.isa != ISA_MIPS1  \
> -   || mips_opts.arch == CPU_R3900)
> +#define gpr_interlocks                                \
> +  (mips_opts.isa != ISA_MIPS1                         \
> +   || mips_opts.arch == CPU_R3900                     \
> +   || mips_opts.micromips                             \
> +   )
>  
>  /* Whether the processor uses hardware interlocks to avoid delays
>     required by coprocessor instructions, and thus does not require
> @@ -512,6 +534,7 @@ static int mips_32bitmode = 0;
>      && mips_opts.isa != ISA_MIPS2                     \
>      && mips_opts.isa != ISA_MIPS3)                    \
>     || mips_opts.arch == CPU_R4300                     \
> +   || mips_opts.micromips                             \
>     )
>  
>  /* Whether the processor uses hardware interlocks to protect reads
> @@ -519,7 +542,10 @@ static int mips_32bitmode = 0;
>     thus does not require nops to be inserted.  This applies to
>     instructions marked INSN_COPROC_MEMORY_DELAY.  These nops are only
>     requires at MIPS ISA level I.  */
> -#define cop_mem_interlocks (mips_opts.isa != ISA_MIPS1)
> +#define cop_mem_interlocks                            \
> +  (mips_opts.isa != ISA_MIPS1                         \
> +   || mips_opts.micromips                             \
> +   )
>  
>  /* Is this a mfhi or mflo instruction?  */
>  #define MF_HILO_INSN(PINFO) \

These changes are OK if they make life easier, but please add a comment
saying why they do.

> +#define RELAX_MICROMIPS_ENCODE(type, is_16bit, uncond, link, toofar)	\
> +  (0x40000000							\
> +   | ((type) & 0xff)						\
> +   | ((is_16bit) ? 0x100 : 0)					\
> +   | ((uncond) ? 0x200 : 0)					\
> +   | ((link) ? 0x400 : 0)					\
> +   | ((toofar) ? 0x800 : 0))
> +#define RELAX_MICROMIPS_P(i) (((i) & 0xc0000000) == 0x40000000)
> +#define RELAX_MICROMIPS_TYPE(i) ((i) & 0xff)
> +#define RELAX_MICROMIPS_USER_16BIT(i) (((i) & 0x100) != 0)
> +#define RELAX_MICROMIPS_UNCOND(i) (((i) & 0x200) != 0)
> +#define RELAX_MICROMIPS_LINK(i) (((i) & 0x400) != 0)
> +#define RELAX_MICROMIPS_TOOFAR(i) (((i) & 0x800) != 0)
> +#define RELAX_MICROMIPS_MARK_TOOFAR(i) ((i) | 0x800)
> +#define RELAX_MICROMIPS_CLEAR_TOOFAR(i) ((i) & ~0x800)

Is there a need to create variant frags when the user has explicitly
specified the instruction size?  I wouldn't have expected any relaxation
to be necessary in that case, and it looks like the relaxation code does
indeed return 2 whenever USER_16BIT is true.

If the bit really is needed, let's call the parameter "user_16bit"
rather than "is_16bit", to match the use.

Add a comment saying what RELAX_MICROMIPS_TYPE is.  See the MIPS16
comment as an example.

> +#define RELAX_MICROMIPS_EXTENDED(i) (((i) & 0x10000) != 0)
> +#define RELAX_MICROMIPS_MARK_EXTENDED(i) ((i) | 0x10000)
> +#define RELAX_MICROMIPS_CLEAR_EXTENDED(i) ((i) & ~0x10000)

Any particular reason why 0x10000 rather than 0x1000, which seems
to be the first unused bit?  I would prefer to pack the used bits
together so that it's easier to tell what's left.

> +  /* True if the macro is in a 16-bit branch delay slot.  */
> +  bfd_boolean delay_slot_16bit_p;
> +
> +  /* True if the macro is in a 32-bit branch delay slot.  */
> +  bfd_boolean delay_slot_32bit_p;

I think it would be cleaner to have:

  /* If the macro is in a delay slot that requires a specific length
     of instruction, this is that length, otherwise it is zero.  */
  unsigned int delay_slot_length;

> +  /* For relaxable macros, fsize[0] is the length of the first instruction
> +     of the first alternative in bytes and fsize[1] is the length of the
> +     first instruction of the second alternative.
> +     For non-relaxable macros, both elements give the length of the
> +     first instruction in bytes.  */
> +  unsigned int fsize[2];

Rename to "first_insn_sizes" ("sizes" rather than "size" because the
plurality comes from having two alternatives).  Also add:

  The fields are zero if we haven't yet seen the first instruction.

> @@ -2374,22 +2736,21 @@ s_is_linkonce (symbolS *sym, segT from_s
>    return linkonce;
>  }
>  
> -/* Mark instruction labels in mips16 mode.  This permits the linker to
> -   handle them specially, such as generating jalx instructions when
> -   needed.  We also make them odd for the duration of the assembly, in
> -   order to generate the right sort of code.  We will make them even
> +/* Mark instruction labels in MIPS16/microMIPS mode.  This permits the
> +   linker to handle them specially, such as generating jalx instructions
> +   when needed.  We also make them odd for the duration of the assembly,
> +   in order to generate the right sort of code.  We will make them even
>     in the adjust_symtab routine, while leaving them marked.  This is
>     convenient for the debugger and the disassembler.  The linker knows
>     to make them odd again.  */
>  
>  static void
> -mips16_mark_labels (void)
> +mips_compressed_mark_labels (void)
>  {
>    segment_info_type *si = seg_info (now_seg);
>    struct insn_label_list *l;
>  
> -  if (!mips_opts.mips16)
> -    return;
> +  gas_assert (HAVE_CODE_COMPRESSION);
>  
>    for (l = si->label_list; l != NULL; l = l->next)
>     {
> @@ -2397,16 +2758,22 @@ mips16_mark_labels (void)
>  
>  #if defined(OBJ_ELF) || defined(OBJ_MAYBE_ELF)
>        if (IS_ELF)
> -	S_SET_OTHER (label, ELF_ST_SET_MIPS16 (S_GET_OTHER (label)));
> +	{
> +	  if (mips_opts.mips16)
> +	    S_SET_OTHER (label, ELF_ST_SET_MIPS16 (S_GET_OTHER (label)));
> +	  else if (mips_opts.micromips)
> +	    S_SET_OTHER (label, ELF_ST_SET_MICROMIPS (S_GET_OTHER (label)));
> +	}
>  #endif
>        if ((S_GET_VALUE (label) & 1) == 0
>  	/* Don't adjust the address if the label is global or weak, or
>  	   in a link-once section, since we'll be emitting symbol reloc
>  	   references to it which will be patched up by the linker, and
> -	   the final value of the symbol may or may not be MIPS16.  */
> +	   the final value of the symbol may or may not be MIPS16/microMIPS.  */
>  	  && ! S_IS_WEAK (label)
>  	  && ! S_IS_EXTERNAL (label)
> -	  && ! s_is_linkonce (label, now_seg))
> +	  && ! s_is_linkonce (label, now_seg)
> +	  && HAVE_CODE_COMPRESSION)
>  	S_SET_VALUE (label, S_GET_VALUE (label) | 1);
>      }
>  }

Looks like the addition of HAVE_CODE_COMPRESSION is redundant here,
you've already asserted it in the previous hunk.

> +static char *
> +micromips_label_name (void)
> +{
> +  char *p = micromips_target_name;
> +  char symbol_name_temporary[24];
> +  unsigned long l;
> +  int i;
> +
> +  if (*p)
> +    return p;
> +
> +  i = 0;
> +  l = micromips_target_label;
> +#ifdef LOCAL_LABEL_PREFIX
> +  *p++ = LOCAL_LABEL_PREFIX;
> +#endif
[...]
> +int
> +mips_label_is_local (const char *name)
> +{
> +  return strchr (name, MICROMIPS_LABEL_CHAR) != NULL;
> +}

Why is this change needed?  The default local-label detection should be
enough for ELF targets, which always have a LOCAL_LABEL_PREFIX.

If we do need TC_LABEL_IS_LOCAL for some reason, it should be documented
in internals.texi.

> @@ -2915,7 +3367,8 @@ append_insn (struct mips_cl_insn *ip, ex
>  	 out that the branch was out-of-range, we'll get an error.  */
>        && !mips_opts.warn_about_macros
>        && (mips_opts.at || mips_pic == NO_PIC)
> -      && !mips_opts.mips16)
> +      && !mips_opts.mips16
> +      && !mips_opts.micromips)
>      {
>        relaxed_branch = TRUE;
>        add_relaxed_insn (ip, (relaxed_branch_length

!HAVE_CODE_COMPRESSION

> +      if (mips_relax.sequence)
> +	abort ();

gas_assert (!mips_relax.sequence);

> +	      /* For microMIPS, disable reordering.  */
> +	      || mips_opts.micromips

You should say whether this is for simplicity or by specification.
Either way, a bit more detail would be welcome.  E.g. something like:

	      /* microMIPS assembly language does not allow the assembler
	      	 to reorder instructions, even in .set reorder mode.
		 Delay slots are always filled with nops when .set reorder
		 is in effect.  */

(adjusted as appropriate if my guess is wrong).

> +  /* If both delay slots are out of size, then emit the warning now.  */
> +  if ((subtype & (RELAX_DELAY_SLOT_SIZE_FIRST | RELAX_DELAY_SLOT_SIZE_SECOND))
> +      == (RELAX_DELAY_SLOT_SIZE_FIRST | RELAX_DELAY_SLOT_SIZE_SECOND))

  /* If both alternatives fail to fill a delay slot correctly,
     emit the warning now.  */
  if ((subtype & RELAX_DELAY_SLOT_SIZE_FIRST) != 0
      && (subtype & RELAX_DELAY_SLOT_SIZE_SECOND) != 0)

> +  hash = !mips_opts.micromips ? op_hash : micromips_op_hash;

  hash = mips_opts.micromips ? micromips_op_hash : op_hash;

> @@ -3640,13 +4407,32 @@ macro_build (expressionS *ep, const char
>        /* Search until we get a match for NAME.  It is assumed here that
>  	 macros will never generate MDMX, MIPS-3D, or MT instructions.  */
>        if (strcmp (fmt, mo->args) == 0
> -	  && mo->pinfo != INSN_MACRO
> -	  && is_opcode_valid (mo))
> -	break;
> +	  && mo->pinfo != INSN_MACRO)
> +	{
> +	  bfd_boolean ok;
> +	  bfd_boolean size_ok;
> +	  bfd_boolean delay_slot_ok;
> +
> +	  ok = is_opcode_valid (mo);
> +	  size_ok = is_size_valid (mo);
> +	  delay_slot_ok = is_delay_slot_valid (mo);
> +	  if (ok && size_ok && (delay_slot_ok || secondpass))
> +	    break;
> +	  if (!delay_slot_ok && !twopass)
> +	    {
> +	      firstmo = mo;
> +	      twopass = TRUE;
> +	    }
> +	}
>  
>        ++mo;
> -      gas_assert (mo->name);
> -      gas_assert (strcmp (name, mo->name) == 0);
> +      if (!mo->name || strcmp (name, mo->name) != 0)
> +	{
> +	  gas_assert (twopass);
> +	  gas_assert (!secondpass);
> +	  secondpass = TRUE;
> +	  mo = firstmo;
> +	}
>      }
>  
>    create_insn (&insn, mo);

Do we really need to do two passes here?  I would have expected
to see something like:

      if (strcmp (fmt, mo->args) == 0
          && mo->pinfo != INSN_MACRO
          && is_opcode_valid (mo)
          && is_size_valid (mo))
        {
          if (is_delay_slot_valid (mo))
            break;
          else if (!reserve_mo)
            reserve_mo = mo;
        }

      if (!mo->name || strcmp (name, mo->name) != 0)
        {
          /* All usable candidates violate the delay slot requirements
	     of the previous instruction.  Pick the first such candidate
	     anyway; we will issue an appropriate warning later.  */
          gcc_assert (reserve_mo);
          mo = reserve_mo;
          break;
        }

which IMO is simpler and clearer.

> +	  if (!mips_opts.micromips)
> +	    INSERT_OPERAND (0, RD, insn, va_arg (args, int));
> +	  else
> +	    INSERT_OPERAND (1, RS, insn, va_arg (args, int));

	  if (mips_opts.micromips)
	    INSERT_OPERAND (1, RS, insn, va_arg (args, int));
	  else
	    INSERT_OPERAND (0, RD, insn, va_arg (args, int));

>  	case 'i':
>  	case 'j':
>  	  macro_read_relocs (&args, r);
> -	  gas_assert (*r == BFD_RELOC_GPREL16
> +	  gas_assert (mips_opts.micromips
> +		      || *r == BFD_RELOC_GPREL16
>  		      || *r == BFD_RELOC_MIPS_HIGHER
>  		      || *r == BFD_RELOC_HI16_S
>  		      || *r == BFD_RELOC_LO16
>  		      || *r == BFD_RELOC_MIPS_GOT_OFST);
> +	  gas_assert (!mips_opts.micromips
> +		      || *r == BFD_RELOC_MICROMIPS_GPREL16
> +		      || *r == BFD_RELOC_MICROMIPS_HIGHER
> +		      || *r == BFD_RELOC_MICROMIPS_HI16_S
> +		      || *r == BFD_RELOC_MICROMIPS_LO16
> +		      || *r == BFD_RELOC_MICROMIPS_GOT_OFST);

Let's move the macro_read_relocs stuff inside append_insn rather than
leaving the conversion to the callers.  You could then make append_insn
keep a record of the original (non-microMIPS) reloc_type[0] too,
which would simply some of the logic.  E.g. these changes would
no longer be needed:

> -	  /* Tag symbols that have a R_MIPS16_26 relocation against them.  */
> -	  if (reloc_type[0] == BFD_RELOC_MIPS16_JMP
> +	  /* Tag symbols that have a R_MIPS16_26 or R_MICROMIPS_26_S1
> +	     relocation against them.  */
> +	  if ((reloc_type[0] == BFD_RELOC_MIPS16_JMP
> +	       || reloc_type[0] == BFD_RELOC_MICROMIPS_JMP)
>  	      && ip->fixp[0]->fx_addsy)
>  	    *symbol_get_tc (ip->fixp[0]->fx_addsy) = 1;
> @@ -3105,6 +3638,13 @@ append_insn (struct mips_cl_insn *ip, ex
>  		  || reloc_type[0] == BFD_RELOC_MIPS_SCN_DISP
>  		  || reloc_type[0] == BFD_RELOC_MIPS_REL16
>  		  || reloc_type[0] == BFD_RELOC_MIPS_RELGOT
> +		  || reloc_type[0] == BFD_RELOC_MICROMIPS_JMP
> +		  || reloc_type[0] == BFD_RELOC_MICROMIPS_GPREL16
> +		  || reloc_type[0] == BFD_RELOC_MICROMIPS_LITERAL
> +		  || reloc_type[0] == BFD_RELOC_MICROMIPS_SUB
> +		  || reloc_type[0] == BFD_RELOC_MICROMIPS_HIGHEST
> +		  || reloc_type[0] == BFD_RELOC_MICROMIPS_HIGHER
> +		  || reloc_type[0] == BFD_RELOC_MICROMIPS_SCN_DISP
>  		  || reloc_type[0] == BFD_RELOC_MIPS16_GPREL
>  		  || hi16_reloc_p (reloc_type[0])
>  		  || lo16_reloc_p (reloc_type[0])))

You also wouldn't need micromips_percent_op.

Sorry, I know this isn't what I said first time round, but it was much
harder to see the wood for the trees before you'd integrated the macro
code properly.

> +  /* For microMIPS, check if the current instruction is not in
> +     a delay slot that requires a 32-bit instruction.  */
> +  if (mips_opts.micromips
> +      && !(history[0].insn_mo->pinfo2 & INSN2_BRANCH_DELAY_32BIT))

  /* Prefer to use a 16-bit microMIPS instruction unless the previous
     instruction specifically requires a 32-bit one.  */

> +   if CALL is set.  In the reorder mode the delay slot would be filled
> +   with a nop anyway, so code produced is simply:
> +	BR	<args>, <sym>

Add an explicit nop, since the code does.

> +/* Emit a coprocessor branch macro specified by TYPE, using CC as
> +   the condition code tested.  EP specifies the branch target.  */

"branch-likely macro".

> +/* Emit a two-argument branch macro specified by TYPE, using SREG as
> +   the register tested.  EP specifies the branch target.  */
> +
> +static void
> +macro_build_branch_rs (int type, expressionS *ep, unsigned int sreg)
> +{
> +  const char *brneg;
> +  const char *br;
> +  int likely = 0;
> +  int call = 0;
> +
> +  switch (type)
> +    {
> +    case M_BGEZ:
> +      br = "bgez";
> +      break;
> +    case M_BGEZL:
> +      br = mips_opts.micromips ? "bgez" : "bgezl";
> +      brneg = "bltz";
> +      likely = 1;
> +      break;
> +    case M_BGEZALL:
> +      gas_assert (mips_opts.micromips);
> +      br = "bgezals";
> +      brneg = "bltz";
> +      likely = 1;
> +      call = 1;
> +      break;
> +    case M_BGTZ:
> +      br = "bgtz";
> +      break;
> +    case M_BGTZL:
> +      br = mips_opts.micromips ? "bgtz" : "bgtzl";
> +      brneg = "blez";
> +      likely = 1;
> +      break;
> +    case M_BLEZ:
> +      br = "blez";
> +      break;
> +    case M_BLEZL:
> +      br = mips_opts.micromips ? "blez" : "blezl";
> +      brneg = "bgtz";
> +      likely = 1;
> +      break;
> +    case M_BLTZ:
> +      br = "bltz";
> +      break;
> +    case M_BLTZL:
> +      br = mips_opts.micromips ? "bltz" : "bltzl";
> +      brneg = "bgez";
> +      likely = 1;
> +      break;
> +    case M_BLTZALL:
> +      gas_assert (mips_opts.micromips);
> +      br = "bltzals";
> +      brneg = "bgez";
> +      likely = 1;
> +      call = 1;
> +      break;
> +    default:
> +      abort ();
> +    }
> +  if (mips_opts.micromips && likely)
> +    macro_build_branch_likely (br, brneg, call, ep, "s,p", sreg, ZERO);
> +  else
> +    macro_build (ep, br, "s,p", sreg);
> +}

No need for "likely".  Just initialise "brneg" to NULL and check for that.
Same for macro_build_branch_rsrt.

> +      if (!mips_opts.micromips)
> +	label_expr.X_add_number = 8;
> +      else
> +	micromips_label_expr (&label_expr);

      if (mips_opts.micromips)
	micromips_label_expr (&label_expr);
      else
	label_expr.X_add_number = 8;

(several occurences)

> -	  macro_build (NULL, "jalr", "d,s", dreg, sreg);
> +	  s = (!mips_opts.micromips || (mips_opts.noreorder && !cprestore)
> +	       ? "jalr" : "jalrs");
> +	  if (mips_opts.micromips && dreg == RA)
> +	    macro_build (NULL, s, "mj", sreg);
> +	  else
> +	    macro_build (NULL, s, JALR_FMT, dreg, sreg);

Since we can use JALRS for mips_opts.noreorder && cprestore, I suppose
the cprestore nop:

 		  if (mips_opts.noreorder)
		    macro_build (NULL, "nop", "");

ought to be conditional on !mips_opts.micromips.

> +	  /* A 12-bit offset field is too narrow to be used for a low-part
> +	     relocation, so use the auxiliary register to load the full
> +	     address provided if the A(b) format has been requested;
> +	     load_address will produce the necessary relocations as code
> +	     used with 16-bit offsets below would do.  Otherwise the o(b)
> +	     format has been selected, so load the low part only and the
> +	     relocation requested will have already been provided in
> +	     offset_reloc, so just use that.  */

	  /* A 12-bit offset field is too narrow to be used for a low-part
	     relocation, so load the whole address into the auxillary
	     register.  In the case of "A(b)" addresses, we first load
	     absolute address "A" into the register and then add base
	     register "b".  In the case of "o(b)" addresses, we simply
	     need to add 16-bit offset "o" to base register "b", and
	     offset_reloc already contains the relocations associated
	     with "o".  */

> +  hash = !mips_opts.micromips ? op_hash : micromips_op_hash;
> +  past = (!mips_opts.micromips ? &mips_opcodes[NUMOPCODES]
> +	  : &micromips_opcodes[bfd_micromips_num_opcodes]);

  if (mips_opts.micromips)
    {
      hash = micromips_op_hash;
      past = &micromips_opcodes[bfd_micromips_num_opcodes];
    }
  else
    {
      hash = op_hash;
      past = &mips_opcodes[NUMOPCODES];
    }

> @@ -8688,32 +10234,50 @@ mips_ip (char *str, struct mips_cl_insn 
>    argsStart = s = str + end;
>    for (;;)
>      {
> +      bfd_boolean delay_slot_ok;
> +      bfd_boolean size_ok;
>        bfd_boolean ok;
>  
>        gas_assert (strcmp (insn->name, name) == 0);
>  
>        ok = is_opcode_valid (insn);
> -      if (! ok)
> +      size_ok = is_size_valid (insn);
> +      delay_slot_ok = is_delay_slot_valid (insn);
> +      if (!delay_slot_ok && !twopass)
>  	{
> -	  if (insn + 1 < &mips_opcodes[NUMOPCODES]
> -	      && strcmp (insn->name, insn[1].name) == 0)
> +	  firstinsn = insn;
> +	  twopass = TRUE;
> +	}
> +      if (!ok || !size_ok || (!delay_slot_ok && !secondpass))
> +	{
> +	  static char buf[256];
> +
> +	  if (insn + 1 < past && strcmp (insn->name, insn[1].name) == 0)
>  	    {
>  	      ++insn;
>  	      continue;
>  	    }
> -	  else
> +	  if (twopass && !secondpass)
>  	    {
> -	      if (!insn_error)
> -		{
> -		  static char buf[100];
> -		  sprintf (buf,
> -			   _("opcode not supported on this processor: %s (%s)"),
> -			   mips_cpu_info_from_arch (mips_opts.arch)->name,
> -			   mips_cpu_info_from_isa (mips_opts.isa)->name);
> -		  insn_error = buf;
> -		}
> -	      return;
> +	      gas_assert (firstinsn);
> +	      secondpass = TRUE;
> +	      insn = firstinsn;
> +	      continue;
>  	    }
> +
> +	  if (insn_error)
> +	    return;
> +
> +	  if (!ok)
> +	    sprintf (buf, _("opcode not supported on this processor: %s (%s)"),
> +		     mips_cpu_info_from_arch (mips_opts.arch)->name,
> +		     mips_cpu_info_from_isa (mips_opts.isa)->name);
> +	  else
> +	    sprintf (buf, _("Unrecognized %u-bit version of microMIPS opcode"),
> +		     8 * forced_insn_length);
> +	  insn_error = buf;
> +
> +	  return;
>  	}
>  
>        create_insn (ip, insn);

Same two-pass comment as before.

> +		unsigned long mask = (!mips_opts.micromips
> +				      ? OP_MASK_CODE
> +				      : MICROMIPSOP_MASK_CODE);


		unsigned long mask = (mips_opts.micromips
				      ? MICROMIPSOP_MASK_CODE
				      : OP_MASK_CODE);

Several other cases.

> +	    case 'J':
> +	      if (!mips_opts.micromips)
> +		{		/* 19-bit WAIT code.  */
> +		  my_getExpression (&imm_expr, s);
> +		  check_absolute_expr (ip, &imm_expr);
> +		  if ((unsigned long) imm_expr.X_add_number > OP_MASK_CODE19)
> +		    {
> +		      as_warn (_("Illegal 19-bit code (%lu)"),
> +			       (unsigned long) imm_expr.X_add_number);
> +		      imm_expr.X_add_number &= OP_MASK_CODE19;
> +		    }
> +		  INSERT_OPERAND (0, CODE19, *ip, imm_expr.X_add_number);
> +		  imm_expr.X_op = O_absent;
> +		  s = expr_end;
> +		  continue;
>  		}
> -	      INSERT_OPERAND (CODE19, *ip, imm_expr.X_add_number);
> -	      imm_expr.X_op = O_absent;
> -	      s = expr_end;
> -	      continue;
> +	      goto do_reg;	/* ALNV.PS source register.  */

So 'J' is used for different things depending on micromips mode?
Couldn't we use a different letter instead?

> +		      if (!mips_opts.micromips)
> +			INSERT_OPERAND (0, RD, *ip, regno);
> +		      else
> +			INSERT_OPERAND (1, RS, *ip, regno);

		      if (mips_opts.micromips)
			INSERT_OPERAND (1, RS, *ip, regno);
		      else
			INSERT_OPERAND (0, RD, *ip, regno);

> +		  if (!ok)
> +		    {
> +		      switch (*args++)

I realise you've copied this from elsewhere, but why "++"?
The "for" loop increments "args", doesn't it?

> +		      if (c == 'e')
> +			{
> +			  regno = lastregno;
> +			  s = s_reset;
> +			  ++args;
> +			}
> +		      else if (c == 't')
> +			{
> +			  s = s_reset;
> +			  ++args;
> +			  continue;
> +			}

I don't really understand these "args" adjustments either.

> +		    i = my_getSmallExpression (&imm_expr, imm_reloc, s);
> +		    if ((i == 0 && (imm_expr.X_op != O_constant
> +				    || (imm_expr.X_add_number & 3) != 0
> +				    || imm_expr.X_add_number > (63 << 2)
> +				    || imm_expr.X_add_number < (-64 << 2)))
> +			|| i > 0)
> +		      {
> +			imm_expr.X_op = O_absent;
> +			break;
> +		      }
> +		    immed = imm_expr.X_add_number >> 2;
> +		    INSERT_OPERAND (1, IMMA, *ip, immed);
> +		    imm_expr.X_op = O_absent;
> +		    s = expr_end;
> +		    continue;

Why set X_op to O_absent when rejecting this alternative?  What breaks
if you leave the constant in imm_expr?  I couldn't see any similar
error-handling code in this function.

Also:

		    if (my_getSmallExpression (&imm_expr, imm_reloc, s) == 0
			&& imm_expr.X_op == O_constant
			&& (imm_expr.X_add_number & 3) == 0
			&& imm_expr.X_add_number >= (-64 << 2)
			&& imm_expr.X_add_number <= (63 << 2))
		      {
			immed = imm_expr.X_add_number >> 2;
			INSERT_OPERAND (1, IMMA, *ip, immed);
			imm_expr.X_op = O_absent;
			s = expr_end;
			continue;
		      }
		    break;

seems more in keeping with other my_getSmallExpression users.

> +		    i = my_getSmallExpression (&imm_expr, imm_reloc, s);
> +
> +		    for (immb = 0; immb < 8; immb++)
> +		      {
> +			if (micromips_imm_b_map[immb]
> +			    == imm_expr.X_add_number)
> +			  break;
> +		      }
> +		    if ((i == 0 && (imm_expr.X_op != O_constant || immb == 8))
> +			|| i > 0)
> +		      {
> +			imm_expr.X_op = O_absent;
> +			break;
> +		      }
> +		    INSERT_OPERAND (1, IMMB, *ip, immb);
> +		    imm_expr.X_op = O_absent;
> +		    s = expr_end;
> +		    continue;

Here too I'd prefer something like:

		    if (my_getSmallExpression (&imm_expr, imm_reloc, s) == 0
			&& imm_expr.X_op == O_constant)
		      {
			for (immb = 0; immb < 8; immb++)
			  if (micromips_imm_b_map[immb]
			      == imm_expr.X_add_number)
			    break;
			if (immb < 8)
			  {
			    INSERT_OPERAND (1, IMMB, *ip, immb);
			    imm_expr.X_op = O_absent;
			    s = expr_end;
			    continue;
			  }
		      }
		    break;

which has the added benefit of only using X_add_number once we've
established that it's meaningful.  Similar changes in the rest
of the function.

> +		  /* If users want relax branch and don't specify to use
> +		     16-bit instructions, we will not match this pattern.
> +		     This will lead to matching 32-bit instructions, that
> +		     will be relaxed later.  */
> +		  if (mips_relax_branch && forced_insn_length != 2)
> +		    break;

This seems a bit lame.  It should be easy to relax the 16-bit form
in the same way as the 32-bit form.  We could use a bit in the
relaxation opcode to say whether the extra relaxation should be
enabled or not, i.e. a bit to record the relevant parts of this
condition:

+	   && (pinfo & INSN_UNCOND_BRANCH_DELAY
+	       || pinfo & INSN_COND_BRANCH_DELAY)
+	   && mips_relax_branch
+	   /* Don't try branch relaxation within .set nomacro, or within
+	      .set noat if we use $at for PIC computations.  If it turns
+	      out that the branch was out-of-range, we'll get an error.  */
+	   && !mips_opts.warn_about_macros
+	   && (mips_opts.at || mips_pic == NO_PIC)
+	   && mips_opts.micromips
+	   /* Don't try branch relaxation, when users specify 16-bit/32-bit
+	      instructions.  */
+	   && !forced_insn_length)

No need to do that as part of this patch, but let's at least put in
a FIXME.

> +		case 'N':	/* Register list for lwm and swm.  */
> +		  {
> +		    unsigned int reg_list = 0;
> +		    int immed = 0;
> +		    unsigned int reg1 = 33;

Why 33 rather than 32?  Seems INVALID_REG could be used here for a bit
of extra readability.

> +		    /* s0, ra
> +		       s0, s1, ra
> +		       s0, s1, s2, ra
> +		       s0, s1, s2, s3, ra
> +		       s0-s1, ra
> +		       s0-s2, ra
> +		       s0-s3, ra */

You also allow:

    s1, ra, s2, s0

(rightly IMO), so I don't think we gain much by listing the split-out ranges.
Just:

		    /* s0, ra
		       s0-s1, ra
		       s0-s2, ra
		       s0-s3, ra */

would be OK.

> +			s_reset = s;
> +			SKIP_SPACE_TABS (s);
> +			if (*s == ',')
> +			  {
> +			    reg1 = 33;
> +			    ++s;
> +			  }
> +			else if (*s == '-')
> +			  {
> +			    reg1 = regno;
> +			    ++s;
> +			  }
> +			SKIP_SPACE_TABS (s);
> +			ok = reg_lookup (&s, RTYPE_NUM | RTYPE_GP, &regno);
> +			if (!ok)
> +			  {
> +			    s = s_reset;
> +			    break;
> +			  }

Shouldn't we break out if *s isn't ',' or '-' and treat it as a syntax
error ("reg_list = 0; break;")?

> +			    if (regno <= reg1)
> +			      {
> +				reg_list = 0;
> +				break;
> +			      }

Add a comment saying that this is an error condition.  But is there
really anything wrong with $4-$4 (regno == reg1)?  The range is closed
after all.

> +		      }
> +		    if (i == 0 && imm_expr.X_op == O_constant
> +			&& (imm_expr.X_add_number & 3) == 0
> +			&& imm_expr.X_add_number >= (-4194304 << 2)
> +			&& imm_expr.X_add_number <= (4194303 << 2))

I'm ashamed to say this decimal number wasn't in my mental power-of-2 list.
Maybe a hex constant or (1 << n) would be better?  Feel free to leave it
if you disagree though.

> +	    case 'n':		/* Register list for 32-bit lwm and swm.  */
> +	      gas_assert (mips_opts.micromips);
> +	      {
> +		unsigned int reg_list = 0;
> +		int immed = 0;
> +		unsigned int reg1 = 33;
> +

This register-range code needs to be split out into a separate function.
It's the same as for 'N' above (except for the s8 handling, which would
be OK-but-redundant for 'n' too), has the same apparent bug regarding
syntax checking, and has the same questionable behaviour about
single-register ranges.

> +		/* s0, ra
> +		   s0, s1, ra
> +		   s0, s1, s2, ra
> +		   s0, s1, s2, s3, ra
> +		   s0, s1, s2, s3, s4, ra
> +		   s0, s1, s2, s3, s4, s5, ra
> +		   s0, s1, s2, s3, s4, s5, s6, ra
> +		   s0, s1, s2, s3, s4, s5, s6, s7, ra
> +		   s0, s1, s2, s3, s4, s5, s6, s7, s8, ra
> +		   ra,
> +		   s0-s1, ra
> +		   s0-s2, ra
> +		   s0-s3, ra
> +		   s0-s4, ra
> +		   s0-s5, ra
> +		   s0-s6, ra
> +		   s0-s7, ra
> +		   s0-s8, ra */

I'm hampered by the spec not being public yet, but it looks from the
code as though RA's actually optional when S0 is present.  The comment
implies otherwise.  Also:

> +		if (reg_list == 0x00010000)
> +		  immed = 1;
> +		else if (reg_list == 0x00030000)
> +		  immed = 2;
> +		else if (reg_list == 0x00070000)
> +		  immed = 3;
> +		else if (reg_list == 0x000f0000)
> +		  immed = 4;
> +		else if (reg_list == 0x001f0000)
> +		  immed = 5;
> +		else if (reg_list == 0x003f0000)
> +		  immed = 6;
> +		else if (reg_list == 0x007f0000)
> +		  immed = 7;
> +		else if (reg_list == 0x00ff0000)
> +		  immed = 8;
> +		else if (reg_list == 0x40ff0000)
> +		  immed = 9;
> +		else if (reg_list == 0x80000000)
> +		  immed = 16;
> +		else if (reg_list == 0x80010000)
> +		  immed = 17;
> +		else if (reg_list == 0x80030000)
> +		  immed = 18;
> +		else if (reg_list == 0x80070000)
> +		  immed = 19;
> +		else if (reg_list == 0x800f0000)
> +		  immed = 20;
> +		else if (reg_list == 0x801f0000)
> +		  immed = 21;
> +		else if (reg_list == 0x803f0000)
> +		  immed = 22;
> +		else if (reg_list == 0x807f0000)
> +		  immed = 23;
> +		else if (reg_list == 0x80ff0000)
> +		  immed = 24;
> +		else if (reg_list == 0xc0ff0000)
> +		  immed = 25;
> +		else
> +		  break;

this could at least be simplified by testing the Sn and RA registers
separately, then rejecting immed == 0.  But maybe the split-out function
could return the Sn range in a more easily digestible form.

> +	  /* For microMIPS we need to save the value to buf + 2.  */
> +	  if (target_big_endian || fixP->fx_r_type == BFD_RELOC_MICROMIPS_LO16)
>  	    buf += 2;

	  /* 32-bit microMIPS instructions are divided into two 16-bit pieces.
	     Relocations always refer to the second piece, regardless of
	     endianness.  */

or something like that.  I'm sure there's a better term than "piece" here,
what does the spec call it?

> +    fragp->fr_subtype
> +      = toofar ? RELAX_MICROMIPS_MARK_TOOFAR (fragp->fr_subtype)
> +	       : RELAX_MICROMIPS_CLEAR_TOOFAR (fragp->fr_subtype);

Non-standard alignment for ":"

    fragp->fr_subtype = (toofar
    		      	 ? RELAX_MICROMIPS_MARK_TOOFAR (fragp->fr_subtype)
			 : RELAX_MICROMIPS_CLEAR_TOOFAR (fragp->fr_subtype);

> +      /* 00000000 <test>:
> +            0:       405e 0006       bgez    $30,10 <test+0x10>
> +            4:       0c00            nop
> +            6:       fc3c 0002       lw      $1,2($28)
> +                             6: R_MIPS_GOT16 .text
> +            a:       3021 0011       addiu   $1,$1,17
> +                             a: R_MIPS_LO16  .text
> +            e:       4721            jalr    $1
> +             ...
> +
> +         00020010 <test2>:
> +            20010:       0c00            nop
> +            20012:       0c00            nop
> +       */
> +
> +      if (!fragp && update > 0)
> +	length += 6;
> +
> +      if (mips_pic != NO_PIC)
> +	{
> +	  /* Additional space for PIC loading of target address.  */
> +	  length += 6;
> +	}
> +
> +      /* If branch is conditional.  */
> +      if (fragp ? !RELAX_MICROMIPS_UNCOND (fragp->fr_subtype) : (update >= 0))
> +	length += 6;
> +    }

This really isn't written very clearly.  The comment has a single
long-branch form, while the code has three if statements.  Say what
you're counting in each case.

> +  /* For microMIPS PC relative relocations, we cannot convert it to
> +     against a section.  If we do, it will mess up the fixp->fx_offset.  */
>    if (fixp->fx_r_type == BFD_RELOC_VTABLE_INHERIT
> -      || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY)
> +      || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY
> +      || fixp->fx_r_type == BFD_RELOC_MICROMIPS_7_PCREL_S1
> +      || fixp->fx_r_type == BFD_RELOC_MICROMIPS_10_PCREL_S1
> +      || fixp->fx_r_type == BFD_RELOC_MICROMIPS_16_PCREL_S1)

"to be against a section".  That's not a helpful comment though.
_How_ will it mess up fixp->fx_offset?  Give the reader a clue why
the problem applies to BFD_RELOC_MICROMIPS_16_PCREL_S1 but not
to something like BFD_RELOC_16_PCREL_S2.

> +	  if (RELAX_MICROMIPS_UNCOND (fragp->fr_subtype))
> +	    goto uncond_micromips;

Ugh.  Either split out the code for unconditional branches into a subfunction,
or put intervening code into a:

	  if (!RELAX_MICROMIPS_UNCOND (fragp->fr_subtype))

block.  (Yes, I know the current relaxation code has the same kind of goto,
but I would have raised just the same objection there.  If cut-&-paste really
is necessary, the new code should still be defendable in its own right.)

Some of the comments below are also about code that has been
cut-&-pasted from the non-microMIPS branch relaxation.  I haven't
flagged them up as such because it's irrelevant.

> +      buf = (bfd_byte *)fragp->fr_literal + fragp->fr_fix;

Missing space before "fragp->fr_literal".  A few other occurences.

> +	  if (RELAX_MICROMIPS_LINK (fragp->fr_subtype))
> +	    {
> +	      /* Clear the and-link bit.  */
> +	      gas_assert ((insn & 0xffa00000) == 0x40200000);
> +
> +	      /* bltzal		0x04100000	bgezal	0x04110000 */
> +	      insn &= ~0x00200000;
> +	    }

Excess cut-&-paste.  The opcodes in the comment are from the normal MIPS
encoding, not the microMIPS one.  (Wondered at first why we were clearing
a bit that, according to the comment, wasn't supposed to be set in the
first place.)

> +	  /* How many bytes in instructions we've already emitted?  */
> +	  i = buf - (bfd_byte *)fragp->fr_literal - fragp->fr_fix;
> +	  /* How many bytes in instructions from here to the end?  */
> +	  i = fragp->fr_var - i;

I don't get this.  "buf" still equals "fragp->fr_literal + fragp->fr_fix",
doesn't it?  We haven't emitted anything yet.  Seems to me that this is
(and should be) the same as "i = fragp->fr_var".

> +	uncond_micromips:
> +	  if (mips_pic == NO_PIC)
> +	    {
> +	      /* j or jal.  */

"j(al) <sym>  R_MICROMIPS_26_S1"

to match the style of the other comments (and to give a bit more info).

> +	      /* lw/ld $at, <sym>($gp)  R_MIPS_GOT16 */
> +	      insn = 0xfc3c0000;
> +	      exp.X_op = O_symbol;
> +	      exp.X_add_symbol = fragp->fr_symbol;
> +	      exp.X_add_number = fragp->fr_offset;

Add the "ld" case or fix the comment.  Also s/R_MIPS_GOT16/R_MICROMIPS_LO16/.

> +	      /* d/addiu $at, $at, <sym>  R_MIPS_LO16 */
> +	      insn = 0x30210000;

Likewise "daddiu".

> +      gas_assert (buf == (bfd_byte *)fragp->fr_literal
> +			 + fragp->fr_fix + fragp->fr_var);

The "+" isn't indented far enough.

> Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips1-fp.d
> ===================================================================
> --- binutils-fsf-trunk-quilt.orig/gas/testsuite/gas/mips/mips1-fp.d	2010-12-07 00:05:05.000000000 +0000
> +++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips1-fp.d	2010-12-07 00:14:47.000000000 +0000
> @@ -9,4 +9,4 @@
>  [0-9a-f]+ <.*>:
>  .*:	46041000 	add.s	\$f0,\$f2,\$f4
>  .*:	44420000 	cfc1	\$2,\$0
> -#pass
> +	\.\.\.
> Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips1-fp.s
> ===================================================================
> --- binutils-fsf-trunk-quilt.orig/gas/testsuite/gas/mips/mips1-fp.s	2010-12-07 00:05:05.000000000 +0000
> +++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips1-fp.s	2010-12-07 00:14:47.000000000 +0000
> @@ -5,3 +5,7 @@
>  foo:
>  	add.s	$f0,$f2,$f4
>  	cfc1	$2,$0
> +
> +# Force at least 8 (non-delay-slot) zero bytes, to make 'objdump' print ...
> +	.align	2
> +	.space	8

Leave out this kind of change.  I realise it's not the style you prefer,
but what's there now is fine.  Same for:

* gas/testsuite/gas/mips/mips32r2-fp32.d
* gas/testsuite/gas/mips/mips64.d

>        msubu   $11, $12
>        mul     $13, $14, $15
>        pref    4, ($16)
> +      .set at
>        pref    4, 32767($17)
>        pref    4, -32768($18)
> +      .set noat
>        ssnop
>  
>  
>        # privileged instructions
>  
>        cache   5, ($1)
> +      .set at
>        cache   5, 32767($2)
>        cache   5, -32768($3)
> -      .set at
>        cache   5, 32768($4)
>        cache   5, -32769($5)
>        cache   5, 32768

We should really have separate noat tests for the prefs and caches
that are no longer in a noat block (a bit like you did for the
wait and sdbbp tests whose immediates have changed).

> -#define STO_MIPS_PLT		0x8
> +#define STO_MIPS_PLT		(1 << 3)

Don't change the definitions of the existing constants; use hex constants
for the new stuff instead.

>  /* This value is used to mark PIC functions in an object that mixes
> -   PIC and non-PIC.  */
> -#define STO_MIPS_PIC		0x20
> -#define ELF_ST_IS_MIPS_PIC(OTHER) \
> -  (((OTHER) & ~ELF_ST_VISIBILITY (-1)) == STO_MIPS_PIC)
> -#define ELF_ST_SET_MIPS_PIC(OTHER) \
> -  (STO_MIPS_PIC | ELF_ST_VISIBILITY (OTHER))
> +   PIC and non-PIC.  Note this bit overlaps with STO_MIPS16.  */
> +#define STO_MIPS_PIC		(1 << 5)
> +#define ELF_ST_IS_MIPS_PIC(other) (((other) & STO_MIPS_FLAGS) == STO_MIPS_PIC)
> +#define ELF_ST_SET_MIPS_PIC(other) (((other) & ~STO_MIPS_FLAGS) | STO_MIPS_PIC)

/* This value is used to mark PIC functions in an object that mixes
   PIC and non-PIC.  Note that this bit overlaps with STO_MIPS16,
   although MIPS16 symbols are never considered to be MIPS_PIC.  */

> +/* Whether code compression (either of the MIPS16 or the microMIPS ASEs)
> +   has been indicated for a .text symbol.  */
> +#define ELF_ST_IS_COMPRESSED(other) \
> +  (ELF_ST_IS_MIPS16(other) || ELF_ST_IS_MICROMIPS(other))

The last line is missing a space before each "(other)".

> +/* microMIPS placeholders.  */

Should be a bit more descriptive.  E.g.:

/* Every MICROMIPSOP_X definition requires a corresponding OP_X
   definition, and vice versa.  This simplifies various parts
   of the operand handling in GAS.  The fields below only exist in
   the microMIPS encoding, so define each one to have an empty range.  */

if indeed that's accurate.

> +/* These are the bitmasks and shift counts used for the different
> +   fields in the instruction formats.  Other than OP, no masks are
> +   provided for the fixed portions of an instruction, since they are
> +   not needed.  */

Seems like too much cut-&-paste: there isn't an OP field here.
"Other than TARGET", perhaps, unless there are other opcode masks here.

> +/* MIPS placeholders.  */

/* Placeholders for fields that only exist in the traditional 32-bit
   instruction encoding; see the comment above for details.  */

> +   "mg" 3-bit MIPS registers 2-7, 16, 17 (MICROMIPSOP_*_MG) at bit 0
> +   "mg" 3-bit MIPS registers 2-7, 16, 17 (MICROMIPSOP_*_MG) at bit 0

Doubled line.

> -      Elf_Internal_Ehdr *header;
> +      Elf_Internal_Ehdr *header = elf_elfheader (info->section->owner);
>  
> -      header = elf_elfheader (info->section->owner);

What's there now is fine.

> +			else
> +			    iprintf (is, "UNKNOWN");

Excess indentation.

> +		     In microMIPS, we need to match instructions (mfc0, mtc0)
> +		     by hand.  */

Something like:

		     The microMIPS encoding does not have a coprocessor
		     identifier field as such, so we must work out the
		     coprocessor number by looking at the opcode.  */

might be more descriptive.

> +/* Return 1 if a symbol associated with the location being disassembled
> +   indicates a compressed mode, either MIPS16 or microMIPS one.  Otherwise,
> +   return 0.  */

Reads more naturally to me without "one".

> +  for (i = 0; i < info->num_symbols; i++)
> +    {
> +      pos = info->symtab_pos + i;
> +
> +      if (bfd_asymbol_flavour (info->symtab[pos]) != bfd_target_elf_flavour)
> +	continue;
> +
> +      symbol = (elf_symbol_type *) info->symtab[pos];
> +      if ((!micromips_ase
> +	   && ELF_ST_IS_MIPS16 (symbol->internal_elf_sym.st_other))
> +	  || (micromips_ase
> +	      && ELF_ST_IS_MICROMIPS (symbol->internal_elf_sym.st_other)))
> +	    return 1;
> +    }

Why is a search necessary here, when the previous code was happy to
look only at the first symbol?  I'm not saying the code is wrong,
but a bit of commentary would be good.

What problem is the ld-lib.exp change fixing?

Richard


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