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

[Peter Ross]

On 25-Aug-1999, David Jeffery <dgj at cs.mu.OZ.AU> wrote:
> On 20-Aug-1999, Peter Ross <petdr at cs.mu.OZ.AU> wrote:
> > 
> > Estimated hours taken: 15
> > 
> > Add preliminary support for memoing type constructors (hash consing)
> 
> You should mention what this support is: you generate a pred to do the
> construction unification for each hash_consed constructor. I guess that
> the intention is to using tabling to get hash_consing for free. Is that
> right? That is what I gather from the diff... and if so it needs to be
> explained somewhere.
> 
This is just what zs asked me to implement, seeing I knew this bit of
the compiler better then him and had some spare time.


> > 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.
> 
> Perhaps hash_cons should be the name of this pragma? Anyhow, you should
> probably add a sentence saying that this hash_cons_C pred is intended to
> replace all construction unifications.
> 

I prefer memo_cons since you are memoing the constructor.


> > 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.
> > 
> > compiler/hlds_out.m:
> > compiler/intermod.m:
> >     Support the memo_cons marker.
> > 
> > compiler/mercury_to_mercury.m:
> >     Handle the pragma memo_cons.
> > 
> > compiler/module_qual.m:
> > compiler/modules.m:
> >     Handle the new type tabled_cons.
> > 
> > compiler/prog_data.m:
> >     Add type tabled_cons which stores the information declared in the
> >     pragma.
> > 
> > compiler/prog_io_pragma.m:
> >     Parse the memo_cons pragma.
> > 
> > compiler/prog_out.m:
> >     Add a new predicate sym_name_to_term, which given a symname
> >     constructs a valid prog_term for that name.
> > 
> > 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).
> 
> I have no idea. Simon?
> 
> 
> 
> 
> > +				{ module_info_incr_errors(Module0, Module) }, 
> > +				prog_out__write_context(Context),
> > +				io__write_string(
> > +					"Error: export status mismatch.\n"),
> 
> This error message stinks.
> 
Any suggestions what to change it to?
I couldn't think of a better message.


> > +				{ 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
> 
> This error message really isn't very helpful either... it just tells you that
> the type is visible in multiple types. It doesn't mention why that is a problem
> (ie. because of the memo_cons pragma). I guess it mentions the line number of
> the pragma, but I still think you could say something like:
> 	Error: pragma memo_cons declaration for constructor `foo/2' which
> 	       appears in multiple types.
> 
OK.

> > +		% XXX Need to handle existential construction
> > +		% unifications correctly.
> > +	{ ExistQVars = [] },
> > +	{ ClassContext = constraints([], []) },
> 
> You definately need to address this XXX. I'll talk to you about this
> personally.
> 
OK.

> > +	% XXX this actually be in the module that handles hashconsing
> 
> s/this actually/this should actually/
> 
> > +	% when it is made.
> > +:- pred make_hashcons_func_name(sym_name::in, arity::in, type_id::in,
> > +		string::out) is det.
> > +
> > +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).
> 
> The mangled name could maybe be a little more robust... I'm not sure what, 
> though.
> 
That issue can be resolved by zs when he starts to use this code.


> > +:- pred construct_types_and_modes(sym_name::in, arity::in,
> > +		type_id::in, hlds_type_defn::in, module_info::in,
> > +		type_and_mode::out, list(type_and_mode)::out) is det.
> 
> Could you please add a comment to this saying what it does. (I worked it
> out after a while, but it deserves a comment).
> 
> > +
> > +construct_types_and_modes(ConsName, Arity, TypeId, TypeDefn,
> > +		ModuleInfo, TypeAndMode, TypesAndModes) :-
> > +	hlds_data__get_type_defn_tparams(TypeDefn, TypeParams),
> > +	construct_type(TypeId, TypeParams, Type),
> > +
> > +	type_util__get_cons_id_arg_types(ModuleInfo, Type,
> > +			cons(ConsName, Arity), TypeArgs),
> > +
> > +	P = (pred(T0::in, T::out) is det :- T = type_only(T0)),
> > +	P(Type, TypeAndMode),
> > +	list__map(P, TypeArgs, TypesAndModes).
> 
> Why don't you add it with the correct modes? You know it is (in, in, ..., out).
> 

That is what modes functions default to anyway.

> > Index: prog_io_pragma.m
> > ===================================================================
> > RCS file: /home/staff/zs/imp/mercury/compiler/prog_io_pragma.m,v
> > retrieving revision 1.21
> > diff -u -r1.21 prog_io_pragma.m
> > --- prog_io_pragma.m	1999/07/13 08:53:24	1.21
> > +++ prog_io_pragma.m	1999/08/19 08:40:49
> > @@ -320,6 +320,13 @@
> >  	parse_tabling_pragma(ModuleName, "minimal_model", eval_minimal, 
> >  		PragmaTerms, ErrorTerm, Result).
> >  
> > +parse_pragma_type(ModuleName, "memo_cons", PragmaTerms, ErrorTerm,
> > +		_VarSet, Result) :-
> > +	parse_simple_pragma(ModuleName, "memo_cons",
> > +		lambda([Name::in, Arity::in, Pragma::out] is det,
> > +			Pragma = tabled_cons(Name, Arity)),
> > +		PragmaTerms, ErrorTerm, Result).
> 
> Just to be picky... you should probably use the new HO syntax here.
> (ie. `pred' rather than `lambda').
> 
cut and paste problem, fixed.

> > Index: prog_out.m
> > ===================================================================
> > RCS file: /home/staff/zs/imp/mercury/compiler/prog_out.m,v
> > retrieving revision 1.43
> > diff -u -r1.43 prog_out.m
> > --- prog_out.m	1999/07/13 08:53:25	1.43
> > +++ prog_out.m	1999/08/19 08:28:12
> > @@ -68,6 +68,12 @@
> >  :- pred prog_out__sym_name_to_string(sym_name, string, string).
> >  :- mode prog_out__sym_name_to_string(in, in, out) is det.
> >  
> > +	% sym_name_to_term(SymName, Args, Term):
> > +	%	convert a symbol name with an associated list of terms
> > +	%	into a term.
> > +:- pred prog_out__sym_name_to_term(sym_name, list(prog_term), prog_term).
> > +:- mode prog_out__sym_name_to_term(in, in, out) is det.
> 
> Isn't there already something like this somewhere else?
> 
Not that I could find.

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