public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@codesourcery.com>
To: <binutils@sourceware.org>
Subject: [PATCH] GAS: Don't shrink the frag size below the default
Date: Thu, 02 Aug 2012 20:16:00 -0000	[thread overview]
Message-ID: <alpine.DEB.1.10.1208021736120.20608@tp.orcam.me.uk> (raw)
Message-ID: <20120802201600.bU17U1idPYFq07lL5Fe9I4oZppvD9zv3XE4E2kNf3NY@z> (raw)

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)
         {

             reply	other threads:[~2012-08-02 19:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-02 19:32 Maciej W. Rozycki [this message]
2012-08-02 20:16 ` Maciej W. Rozycki
2012-08-03  6:51 ` Alan Modra
2012-08-03 22:37   ` Maciej W. Rozycki
2012-08-05  7:16 ` [PATCH] MIPS/GAS: Correct microMIPS branch swapping assertion Richard Sandiford
2012-08-06 21:01   ` Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.1.10.1208021736120.20608@tp.orcam.me.uk \
    --to=macro@codesourcery.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).