Re: [PATCH] microMIPS support

["Maciej W. Rozycki" <macro@codesourcery.com>]

Hi Jan,

> sure I have seen no MIPS arch bugs...

 Yeah...

> > +/* For backwards compatibility we default to MIPS16.  This flag is
> > +   overridden as soon as unambiguous ELF file flags tell us the
> > +   compressed ISA encoding used.  */
> > +static const char mips_compression_mips16[] = "mips16";
> > +static const char mips_compression_micromips[] = "micromips";
> > +static const char *mips_compression_strings[] = {
> 
> nitpick, be more readonly:
> 
> static const char *const mips_compression_strings[] = {

 Hmm, no objection to your suggestion per se, but is it allowed by C89?

> > +  mips_compression_mips16,
> > +  mips_compression_micromips,
> > +  NULL
> > +};
> [...]
> > +/* Return one iff MSYM refers to standard ISA code.  */
> > +
> >  static int
> > -msymbol_is_special (struct minimal_symbol *msym)
> > +msymbol_is_mips (struct minimal_symbol *msym)
> > +{
> > +  return !(MSYMBOL_TARGET_FLAG_1 (msym) | MSYMBOL_TARGET_FLAG_2 (msym));
> 
> (a) | -> ||
> (b) I have found here it is best to expand logical expressions for the patch
>     readers, despite author may see it more logical with the parentheses.
> 
> return !MSYMBOL_TARGET_FLAG_1 (msym) && !MSYMBOL_TARGET_FLAG_2 (msym);

 Bitwise OR usually produces better code that has fewer branches (though 
hosts that support conditional move instructions may limit the impact here 
a bit), here's no harm to evaluate both sides of the expression in all 
cases.

> > @@ -686,8 +779,8 @@ mips_ax_pseudo_register_push_stack (stru
> >    return 0;
> >  }
> >  
> > -/* Table to translate MIPS16 register field to actual register number.  */
> > -static int mips16_to_32_reg[8] = { 16, 17, 2, 3, 4, 5, 6, 7 };
> > +/* Table to translate 3-bit register field to actual register number.  */
> > +static int mips_reg3_to_reg[8] = { 16, 17, 2, 3, 4, 5, 6, 7 };
> 
> It can be const.

 One change at a time.

 This lookup can also be transformed into a simple 
three-assembly-instruction calculation, but it's called in enough places 
that I think it's cheaper in the current form.  But for this to stand this 
table should also be of the "unsigned char" type.

 Thanks for your notes.

  Maciej