This is the mail archive of the binutils@sources.redhat.com 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]

Re: (Patch) fixups for mn10300



  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


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