[m-dev.] for review: per-module string tables

Fergus Henderson fjh at cs.mu.OZ.AU
Thu May 27 19:58:45 AEST 1999


On 26-May-1999, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> Index: compiler/stack_layout.m
> +% will be zero (at which offset one will find an empty string). The string
> +% table is restricted to be small enough to be addressed with 16 bits;
> +% a string is reserved near the start for a string that says "too many
> +% variables". Stack_layout.m will generate a reference to this string
> +% instead of generating an offset that does not fit into 16 bits.
...
> +	stack_layout__lookup_string_in_table("", _, LayoutInfo0, LayoutInfo1),
> +	stack_layout__lookup_string_in_table("TOO_MANY_VARIABLES", _,
> +		LayoutInfo1, LayoutInfo2),

I think it might be better to use something that is clearly not a variable
name, e.g. "<too many variables>", rather than "TOO_MANY_VARIABLES".

> @@ -710,11 +796,16 @@
>  			rval,	% Rval describing the type of a live value.
>  			llds_type, % The llds type of the rval describing the
>  				% type.
> -			rval	% Rval describing the name of a live value.
> -				% Always of llds type string.
> +			rval,	% Rval describing the variable number of a
> +				% live value. Always of llds uint_least16.
> +				% Contains zero if the live value is not
> +				% a variable.

You should document the special meaning of ((uint_least16)-1) for
this field.

> +stack_layout__construct_liveval_name_rvals(var_info(_, LiveValueType),
> +		VarNumRval, VarNameRval, SLI0, SLI) :-
> +	( LiveValueType = var(Var, Name, _, _) ->
> +		term__var_to_int(Var, VarNum0),
> +			% The variable number has to fit into two bytes.
> +			% We reserve the largest such number (Limit)
> +			% to mean that the variable number is too large
> +			% to be represented. This ought not to happen,
> +			% since compilation would be glacial at best
> +			% for procedures with that many variables.
> +		Limit = (1 << (2 * stack_layout__byte_bits)) - 1,
> +		( VarNum0 >= Limit ->
> +			VarNum = Limit
> +		;
> +			VarNum = VarNum0
> +		),

How about replacing those five lines with `int__max(VarNum0, Limit, VarNum)'?

> +/*-------------------------------------------------------------------------*/
> +/*
> +** Definitions for MR_Module_Layout
> +**
> +** The layout struct for a module contains two main components.
> +**
> +** The first is a string table, which contains strings referred to by other
> +** layout structures in the module (initially only the name tables of label
> +** layout structures).

"Name tables" is ambiguous -- do you mean label names, variable names, or both?


Apart from that, that change looks fine.

-- 
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.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------



More information about the developers mailing list