[m-dev.] for review again: reorganize label table, reduce executable sizes

[Fergus Henderson]

Once you've addressed the points mentioned below, please go ahead and
commit this; but I'd still like to see a relative diff showing the
changes you make relative to this diff.

On 02-Jul-1998, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> Index: doc/user_guide.texi
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/doc/user_guide.texi,v
> retrieving revision 1.127
> diff -u -u -r1.127 user_guide.texi
> --- user_guide.texi	1998/07/01 04:10:14	1.127
> +++ user_guide.texi	1998/07/01 04:42:09
> @@ -1185,30 +1185,24 @@
>  @node Time profiling methods
>  @section Time profiling methods
>  
...
> +You can control whether profiling measures
> +real (elapsed) time, user time plus system time, or user time only,
> +by including the options -Tr, -Tp, or -Tv respectively

You should wrap those options up inside `@samp{...}'.

> +in the environment variable MERCURY_OPTIONS
> +when you run the program to be profiled.
> + at c (See the environment variables section below.)

Why is that line commented out?

> -Currently, the @samp{-Tv} and @samp{-Tp} options don't work on Windows,
> -so on Windows you must explicitly specify @samp{-Tr}.

I don't think you should delete that.

> + at sp 1
> + at item MERCURY_OPTIONS
> +A list of options for the Mercury runtime that gets
> +linked into every Mercury program.
> +Their meanings are as follows.
> +
> + at sp 1
> + at table @code
> +
> + at c @item -a
> + at c @item -c
...
> + at c @item -d @var{debugflag}
...
> + at c @item -r
>... +
> + at c @item -t
...
> + at c @item -x
> + at c @item -z @var{area} @var{size}

Can you please include documentation about what these
options do?  I don't mind if it is commented out too,
but if you just mention the letters and don't say what
they do then this doesn't help.

> + at sp 1
> + at item -D @var{debugger}
> +Enables execution tracing of the program,
> +via the internal debugger if @var{debugger} is @samp{i}
> +and via the external debugger if @var{debugger} is @samp{e}.
> + at c The mdb script works by including -Di in MERCURY_OPTIONS.

I think you should uncomment that last sentence;
just put it in parentheses.  The `-Di' should be wrapped
inside `@samp{...}

> + at sp 1
> + at item -p
> +Disables profiling in an executable built in a profiling grade.

I suggest the following instead:

Disables profiling.  This only has an effect if the executable was
built in a profiling grade.

> + at sp 1
> + at item -s @var{area} @var{size}
> +Tells the runtime system to set the size of one area of the runtime system,
> +where @var{area} must be one of @samp{h} (standing for heap),
> + at samp{d} (standing for deterministic stack),
> + at samp{n} (standing for nondeterministic stack)
> +and, in grades with trailing, @samp{t} (standing for trail),
> +to @var{size} kilobytes.

I suggest

   Sets the size of one of the runtime system's memory areas.
   @var{area} must be one of @samp{h} (standing for heap),
   ...
   @var{size} specifies the size of that area, in kilobytes.

> + at sp 1
> + at item -T @var{time_method}
> +If the executable is in a grade that includes time profiling,
> +specifies what time is counted in the profile.

I suggest

	If the executable was compiled in a grade that includes time profiling,
	this option specifies what time is counted in the profile.

runtime/mercury_label.c:
> +#ifdef	MR_NEED_ENTRY_LABEL_ARRAY
> +		entry_array_next = 0;
> +		entry_array_size = INIT_ENTRY_SIZE;
> +		entry_array_sorted = TRUE;
> +		entry_array = malloc(entry_array_size * sizeof(MR_Entry));

You should use checked_malloc() rather than malloc().

> +		return &entry_array[lo-1];

I suggest s/lo-1/lo - 1/

> +#ifdef	MR_NEED_ENTRY_LABEL_INFO
> +extern	void		MR_insert_entry_label(const char *name, Code *addr,
> +				const MR_Stack_Layout_Entry *entry_layout);
> +#else
> +#define	MR_insert_entry_label(n, a, l)	/* nothing */
> +#endif	/* not MR_NEED_ENTRY_LABEL_INFO */
> +
> +#ifdef	MR_NEED_ENTRY_LABEL_ARRAY
> +extern	MR_Entry	*MR_prev_entry_by_addr(const Code *addr);
> +#endif	/* MR_NEED_ENTRY_LABEL_ARRAY */

Please indent the stuff inside #ifdef by two spaces.

runtime/mercury_table.c:
> +void
> +tab_process_all_entries(const Table *table, void f(const void *))
> +{
> +	reg	List	*ptr;
> +	reg	int	i;
> +
> +	for (i = 0; i < table->ta_size; i++) {
> +		for_list (ptr, table->ta_store[i]) {
> +			(*f)(ldata(ptr));
> +		}
> +	}
> +}

If you're going to declare `f' as a function, rather than a function
pointer, then for consistency it would make sense to call it
as a function, rather than as a function pointer.

Cheers,
	Fergus.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.