[PATCH] GAS: Don't shrink the frag size below the default

[Maciej W. Rozycki]

Hi,

 While debugging the MIPS issue recorded here:

http://sourceware.org/ml/binutils/2012-08/msg00043.html

I made an interesting observation as to how the size of frags used to 
store machine code generated is determined.

 As noted in that issue, frags are placed in memory allocated dynamically 
with obstack_alloc.  This is a GNU extension and is implemented in iberty 
and glibc; the former is bundled with binutils and I haven't checked if a 
glibc copy is ever used if used as the system C library.  Obstack memory 
is allocated in chunks and a frag can be freely grown up to the amount of 
memory available in the obstack it has been put in.  Once the frag cannot 
be grown any further it has to be closed and a new frag created, that will 
be placed in a newly allocated obstack.

 The size of the chunk and therefore any obstack allocated, by default, 
depends on the programming model used on the host.  GAS starts by using 
the default size.  This size is calculated as 4096 bytes minus a 
programming-model specific amount calculated based on sizeof (union 
fooround) defined internally, that I believe can be for *nix systems, that 
we may be potentially concerned about, one of 4, 8, 12 or 16, and the 
corresponding amount comes out as a value between 16 and 32, so the 
remaining space is between 4064 and 4080.
 
 Interestingly enough, this size is applied to the first obstack only.  As 
soon as another obstack is needed GAS switches to using about the minimum 
size possible and starts requesting further obstacks in amounts that are 
double the instruction size only, that for the MIPS target means either 4 
or 8 bytes.  How miserable!  Obstack allocation code uses some arbitrary 
look-ahead heuristics and actually provides some more, but that is is 
still 84 bytes only, so beyond the first obstack GAS starts thrashing with 
requests for new obstacks.  This is I believe an unnecessary memory and 
performance waste.

 By how the offending code looks I gather the intent was to provide for 
large obstacks where an excessively big chunk of data needs to be placed 
in a single frag (the associated comment quotes 2GB!), but inadvertently 
the code does not check for the allocation size getting smaller than the 
reasonable default.  There is normally no penalty from using the default 
even for the smallest frags possible, because as many individual frags can 
be placed in a single obstack as will fit there.  Therefore I believe this 
is a missed optimisation.

 Here is a change to address this.  It changes obstack allocation code 
such that the size requested never goes below the default; however it can 
be increased as the caller sees fit to satisfy large pieces of data.

 I have regression-tested it against my current list of targets that is a 
superset of what Alan uses.  No problems triggered.  OK to apply?

2012-08-02  Maciej W. Rozycki  <macro@codesourcery.com>

	gas/
	* frags.c (frag_grow): Never shrink the obstack size requested 
	below the default.

  Maciej

binutils-gas-frag-obstack-chunk-size.diff
Index: binutils-fsf-trunk-quilt/gas/frags.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/frags.c	2011-12-12 23:49:56.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/frags.c	2012-07-09 23:59:16.560713789 +0100
@@ -101,9 +101,11 @@ frag_grow (unsigned int nchars)
       if (newc < 0)
         as_fatal (_("can't extend frag %u chars"), nchars);
 
-      /* Force to allocate at least NEWC bytes.  */
+      /* Force to allocate at least NEWC bytes, but not less than the
+         default.  */
       oldc = obstack_chunk_size (&frchain_now->frch_obstack);
-      obstack_chunk_size (&frchain_now->frch_obstack) = newc;
+      if (newc > oldc)
+	obstack_chunk_size (&frchain_now->frch_obstack) = newc;
 
       while (obstack_room (&frchain_now->frch_obstack) < nchars)
         {