This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: (Patch) fixups for mn10300
- To: Eric Christopher <echristo at cygnus dot com>
- Subject: Re: (Patch) fixups for mn10300
- From: Jeffrey A Law <law at cygnus dot com>
- Date: Thu, 17 Aug 2000 18:30:45 -0600
- cc: binutils at sources dot redhat dot com
- Reply-To: law at cygnus dot com
In message <399BB6E1.E35CE1AF@cygnus.com>you write:
> Here's a patch that will do some fixups for the mn10300 even though the
> linker relaxes (this basically has the end effect of making the symbol
> table smaller in obj files).
>
> Ok to install?
Note we're talking about a major reduction in the size of the symbol table;
without the patch for the following source file we'll get a .o file of
roughly 20k
#include <stdio.h>
And the vast majority of the file is relocs and symbol table entries that
are not needed. With the patch the size of the file is reduced to XXk
Why do we care about this? Well, the linker is IO bound sucking in sections,
relocs, and symbols. By reducing the size of relocs, & symbols and eliminating
the need for sucking in certain sections (like debug sections) we can greatly
reduce the amount of IO the linker has to do and ultimately speed up the
linker.
> 2000-08-17 Eric Christopher <echristo@cygnus.com>
>
> * config/tc-mn10300.c: (md_apply_fix): New function.
> (mn10300_force_relocation): New function.
> (mn10300_fix_adjustable): New function.
> * config/tc-mn10300.h: (TC_FORCE_RELOCATION): Define
> (TC_HANDLES_FX_DONE): Define
> (obj_fix_adjustable): Define
> (MD_APPLY_FIX3): Define
> (TC_LINKRELAX_FIXUP): Define
> * write.c: Define TC_LINKRELAX_FIXUP if not previously
> defined.
> (fixup_segment): Use TC_LINKRELAX_FIXUP.
You need to make a couple minor formatting fixes before checking this in.
> + if (linkrelax
> + && TC_LINKRELAX_FIXUP(this_segment_type))
Always put a space between the macro/function name and the open paren for
its argument list.
> - /* We shouldn't ever get here because linkrelax is nonzero. */
> +
> + valueT value = *valuep;
> + char * fixpos = fix->fx_where + fix->fx_frag->fr_literal;
> + int size = 0;
One space between a type and the variable. Don't try to make the variable
names line up.
> + fix->fx_addnumber = value; /* Remember value for emit_reloc. */
Comment belongs on the previous line.
> +int
> +mn10300_force_relocation (fixp)
> + struct fix * fixp;
This function needs a comment explaining what it does. No space between the
'*' and the variable fixp.
> + if ( fixp->fx_r_type == BFD_RELOC_VTABLE_INHERIT
No spaces after the open parens.
> +boolean
> +mn10300_fix_adjustable (fixp)
> + struct fix * fixp;
Same comments as mn10300_force_relocation.
> + if ( fixp->fx_r_type == BFD_RELOC_VTABLE_INHERIT
> + || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY)
Kill the spaces after the open paren.
I believe it would be useful if you reviewed the GNU coding standards :-)
jeff