Zone allocating collector review

[Geoff Keating <geoffk at geoffk dot org>]

2003-09-09  Daniel Berlin  <dberlin@dberlin.org>

	    * ggc-zone.c:  New file, zone allocating collector.
	    * configure: Accept zone option for --with-gc
	    * configure.in: Ditto.
	    * ggc.h (ggc_pch_count_object): Pass bool indicating
	    stringiness. Update all callers.
	    (ggc_pch_alloc_object): Ditto.
	    (ggc_pch_write_object): Ditto.
	    (ggc_alloc_rtx): Use typed allocation, since all RTX's are
	    * of a single
	    type.
	    (ggc_alloc_rtvec): Ditto.
	    (ggc_alloc_tree): Use zone allocation, since some things
	    * using this macro
	    aren't a single typecode.
	    * ggc-none.c (ggc_alloc_typed): New function.
	    (ggc_alloc_zone): Ditto.

Lots of comments, in no particular order:

- The changes to ggc.h add new parameters to the PCH routines but do
  not change the comments.  Please fix this, those comments are the
  only documentation for these routines.
- Also, those changes add some new functions, but with very unhelpful
  comments, like
  /* The zone internal primitive.  */
  Please try to do better than this.  At least, explain what the
  parameters and result values are.
- In the new added file, ggc-zone.c, some functions are missing
  comments (especially ggc_alloc_typed), which is a serious problem ...
- ... and there is inconsistent whitespace, which isn't so serious but
  is annoying.  I think we've standardised on one blank line between
  the comment and the function.
- If you're never going to set rtl_zone, tree_zone, and garbage_zone
  to be anything other than NULL (as in ggc-page.c and ggc-none.c),
  there's no need to actually say what a 'struct alloc_zone' is.
- Please consider allocating the alloc_zone structures at compile
  time, as static variables.  This will help reduce GCC's startup
  time.  You could even make rtl_zone and friends 'const'.
- It seems like this has a lot of duplicated code with ggc-page.c.
  Do you intend to remove one or the other eventually?  If not,
  you might want to think of ways of reducing the duplication.
- Please check your ATTRIBUTE_UNUSED markers.  In
  ggc_pch_finish and ggc_pch_read, you've marked things as unused that
  are really used.
- The macros 'SCALE' and 'LABEL' appear to be defined but never used.
- The comment above ggc_collect_1 says 
    Returns true if we collected the zone (and thus, performed
    marking), false otherwise.
  but this is untrue; if need_marking is false, marking will not
  be performed although collection is.  The code in ggc_collect
  appears to believe this comment, although as I understand the code
  flow this does not really lead to a bug, just to misleading code,
  since 'marked' will always be true if main_zone.was_collected.
- Why did you introduce ggc_alloc_typed?  It doesn't seem like it's
  really necessary for this implementation.
- You say you bootstrapped & tested with this patch.  Did you also
  use --enable-checking=gcac?  If not, please do so.
- You don't check the return value from the second fwrite in
  ggc_pch_write object.

-- 
- Geoffrey Keating <geoffk@geoffk.org>