[m-dev.] for review: prelim support for hash consing

[Fergus Henderson]

On 20-Aug-1999, Peter Ross <petdr at cs.mu.OZ.AU> wrote:
> Add preliminary support for memoing type constructors (hash consing)

You should explain in what sense this support is preliminary.
What still remains to be done?

Also, the new feature should be documented in appropriate
places (e.g. the Mercury language reference manual).

> compiler/make_hlds.m:
>     The core of the change.
>     For every 
>         :- pragma memo_cons(Constructor/Arity).
>     Construct a function
>         hash_cons_C(X1, ..., XN) = C(X1, ..., XN).
>     where hash_cons_C is a suitably name mangled function.

s/Construct/construct/

> compiler/hlds_pred.m:
>     Add a new marker which indicates that the predicate is an
>     automatically generated predicate which is meant to do a hash consed
>     construction unification.

s/new marker/new marker `memo_cons'/

> diff -u -r1.65 hlds_pred.m
> --- hlds_pred.m	1999/08/18 10:03:00	1.65
> +++ hlds_pred.m	1999/08/20 01:54:51
> @@ -382,6 +382,11 @@
>  				% the termination of this predicate.
>  				% If the compiler cannot guarantee termination
>  				% then it must give an error message.
> +
> +	;	memo_cons
> +				% The predicate should have its
> +				% construction unification use hashcons
> +				% memoing.

That comment is incorrect.  I suggest that you replace it with the
one from the log message.

>  	% Aditi predicates are identified by their owner as well as
> Index: intermod.m
> ===================================================================
> RCS file: /home/staff/zs/imp/mercury/compiler/intermod.m,v
> retrieving revision 1.66
> diff -u -r1.66 intermod.m
> --- intermod.m	1999/07/13 08:53:01	1.66
> +++ intermod.m	1999/08/19 12:53:43
> @@ -1188,6 +1188,8 @@
>  intermod__should_output_marker(generate_inline, _) :-
>  	% This marker should only occur after the magic sets transformation.
>  	error("intermod__should_output_marker: generate_inline").
> +	% XXX is this correct?
> +intermod__should_output_marker(memo_cons, yes).

That is not correct, I think, because the pragma that will be output
by intermod.m will be

	:- pragma memo_cons(hash_cons_C/Arity).

rather than

	:- pragma memo_cons(C/Arity).

> Index: make_hlds.m
...
> +%-----------------------------------------------------------------------------%
> +
> +:- pred module_add_pragma_tabled_cons(sym_name::in, arity::in,
> +		import_status::in, prog_context::in,
> +		module_info::in, module_info::out, 
> +		qual_info::in, qual_info::out,
> +		io__state::di, io__state::uo) is det.
> +
> +module_add_pragma_tabled_cons(Constructor, Arity, Status, Context,
> +		Module0, Module, Info0, Info) -->
> +	{ module_info_ctors(Module0, ConsTable) },
> +	(
> +		{ map__search(ConsTable, cons(Constructor, Arity), Defns) }
> +	->
> +		(
> +			{ Defns = [hlds_cons_defn(_, _, _, TypeId, _)] }
> +		->
> +			{ module_info_types(Module0, Types) },
> +			{ map__lookup(Types, TypeId, TypeDefn) },
> +			{ hlds_data__get_type_defn_status(TypeDefn,
> +					TypeStatus) },
> +			(
> +				{ Status = TypeStatus }
> +			->
> +				module_add_pragma_tabled_cons_2(Constructor,
> +						Arity, TypeId, TypeDefn,
> +						Status, Context,
> +						Module0, Module,
> +						Info0, Info)
> +			;
> +				{ module_info_incr_errors(Module0, Module) }, 
> +				prog_out__write_context(Context),
> +				io__write_string(
> +					"Error: export status mismatch.\n"),
> +				{ Info = Info0 }
> +			)
> +		;
> +			{ module_info_incr_errors(Module0, Module) }, 
> +			prog_out__write_context(Context),
> +			io__write_string("Error: constructor '"),
> +			hlds_out__write_cons_id(cons(Constructor, Arity)),
> +			io__write_string("' in multiple types.\n"),
> +			{ Info = Info0 }
> +
> +			% XXX should list what those types are if -E
> +		)
> +	;
> +		{ module_info_incr_errors(Module0, Module) }, 
> +		prog_out__write_context(Context),
> +		io__write_string("Error: unknown constructor '"),
> +		hlds_out__write_cons_id(cons(Constructor, Arity)),
> +		io__write_string("'.\n"),
> +		{ Info = Info0 }
> +	).

Half of the close-single-quotes (') here should be
open-single-quotes (`).

You should fix that XXX before committing, IMHO.
In fact you should list at least one of the prior declarations,
with an appropriate prog_context, even without `-E'.

But as Zoltan said this is all irrelevant since the pragma
declaration should specify the type, so this error should
be impossible.

> +:- pred module_add_pragma_tabled_cons_2(sym_name::in, arity::in,
> +		type_id::in, hlds_type_defn::in,
> +		import_status::in, prog_context::in,
> +		module_info::in, module_info::out,
> +		qual_info::in, qual_info::out,
> +		io__state::di, io__state::uo) is det.
> +
> +module_add_pragma_tabled_cons_2(ConsName, Arity, TypeId, TypeDefn,
> +		Status, Context, Module0, Module, Info0, Info) -->
> +	{ hlds_data__get_type_defn_tvarset(TypeDefn, TypeVarSet) },
> +	{ varset__init(InstVarSet) },
> +
> +	{ module_info_name(Module0, ModuleName) },
> +	{ make_hashcons_func_name(ConsName, Arity, TypeId, FuncStr) },
> +	{ FuncName = qualified(ModuleName, FuncStr) },
> +
> +	{ construct_types_and_modes(ConsName, Arity, TypeId, TypeDefn,
> +			Module0, ReturnTypeAndMode, TypesAndModes) },
> +
> +	{ MaybeDet = no },

`MaybeDet' should probably be `yes(det)' rather than `no' here.

> +	{ Cond = true },
> +	{ Purity = pure },
> +	{ init_markers(Markers0) },
> +	{ add_marker(Markers0, memo_cons, Markers) },
> +	{ ItemStatus = item_status(Status, must_be_qualified) },
> +
> +		% XXX Need to handle existential construction
> +		% unifications correctly.
> +	{ ExistQVars = [] },
> +	{ ClassContext = constraints([], []) },

That XXX should be fixed before committing too, IMO.
The fix should be straight-forward: just copy the ExistQVars
and ClassContext from the TypeDefn, I think.

> +	% XXX this actually be in the module that handles hashconsing
> +	% when it is made.
> +:- pred make_hashcons_func_name(sym_name::in, arity::in, type_id::in,
> +		string::out) is det.

s/this actually/this should actually/

> +make_hashcons_func_name(ConsName, ConsArity, TypeName - TypeArity, Name) :-
> +	prog_out__sym_name_to_string(ConsName, ConsStr),
> +	prog_out__sym_name_to_string(TypeName, TypeStr),
> +	string__format("cons_%s/%d_%s/%d", [s(TypeStr), i(TypeArity),
> +			s(ConsStr), i(ConsArity)], Name).

If you're going to use a special character like "/"
then you might as well also use spaces rather than underscores.

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