This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] microMIPS support
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