[m-rev.] For review: new constraint based mode analysis

[David Overton]

On Tue, Feb 08, 2005 at 03:58:05PM +1100, Richard James FOTHERGILL wrote:
> 
> For review by anyone.
> 
> Estimated hours taken: 100
> Branches: Main
> 
> These changes provide the framework for a new approach to constraints based
> mode analysis. They build constraints for simple mercury programs (sorry,
> no higher order unifications), and can later be extended to handle all
> applicable hlds goals. They have been designed with the intention that
> they would be solved, for example, by a propagation based constraint solver,
> and the information used for producing a partial order on conjuncts and
> for mode analysis of a mercury program.
> 
> 
> 
> compiler/new_mode_constraints.m

This is not a good name for the module, since it presumably won't be new
for long.

> 	This module serves as an interface between the existing
> 	mode_constraints module which asks for constraints for an SCC, and
> 	build_mode_constraints, which only handles goals.  Simply put, it 
> 	takes
> 	an SCC and hands it to build_mode_constraints one predicate at a 
> 	time.
> 	It produces the mode constraints for that SCC, and the constraint
> 	variables necessary to do so.

There's something wrong with your line wrapping here.

> 
> compiler/options.m
> 	New option inclued --new-mode-constraints. Note that the
> 	new mode constraints material is an offshoot of the original,
> 	so to run it both --mode-constraints and --new-mode-constraints
> 	must be specified.

Not a good name for the option (see my comment on the
new_mode_constraints module name).

> +% This module contains data structures designed for use with constraints
> +% based mode analysis. They represent constraints between constraint 
> variables,
> +% such as those one might use to describe where a program variable may be
> +% produced.
> +%

Please fix up the line wrapping here and in other comments where
necessary.

> +
> +
> +
> +:- module check_hlds.abstract_mode_constraints.
> +
> +:- interface.
> +
> +:- type mc_type.
> +
> +:- import_module term, list, bool, map, counter.
> +:- import_module std_util, varset, io.
> +
> +% XXX change to mc_var throughout

Why is this comment here?

> +	% Represents conjunctions and disjunctions between atomic constraints
> +	% on constraint variables.  The advantage of the constraints for this
> +	% implementation of mode checking is that they can be expressed 
> almost
> +	% entirely as variable to variable constraints, with little use for 
> the
> +	% disj and conj functors of this structure.  It should be noted that
> +	% the intention is to only use the conj constructor as a child of 
> disj

Please clarify what you mean here: is the user of this module not
permitted to use a conj constructor that is not a child of a disj?  If
so, you could enforce this through the type system, e.g:

	:- type constraint_formula
		--->	atomic_constraint(var_constraint)
		;	disj(list(conj_formula)).

	:- type conj_formula
		--->	constraint_formula(constraint_formula)
		;	conj(constraint_formulae).

> +
> +	% Just a conveniently descriptive name.
> +	%
> +:- type args == list(prog_var).
> +
> +
> +	% Just a conveniently descriptive name.
> +	%
> +:- type nonlocals == set(prog_var).

The above two comments are probably not necessary.

> +
> +	% XXX Need to do something here.
> +	%
> +goal_expr_constraints(_ModuleInfo, _VarMap, _PredID,
> +	switch(_, _, _), _GoalPath, _Nonlocals, _Constraints) :-
> +	error("build_mode_constraints.m: "
> +		++ "sorry, switch not implemented.").

Switches are not created until after mode analysis so no problem
throwing an exception here.  However, if you want to be able to rerun
mode analysis after switch detection then you could just treat a switch
the same as a disjuntion.

> +
> +	% Foreign procedure
> +	%
> +goal_expr_constraints(ModuleInfo, VarMap, PredID,
> +	foreign_proc(_, CalledPred, ProcID, ForeignArgs, _, _),
> +	GoalPath, _Nonlocals, Constraints) :-
> +	CallArgs = list.map(foreign_arg_var, ForeignArgs),
> +	module_info_pred_proc_info(ModuleInfo, CalledPred, ProcID, _, 
> ProcInfo),
> +	(	proc_info_maybe_declared_argmodes(ProcInfo, yes(Decl))
> +	->	add_sufficient_in_modes_for_type_info_args(
> +			CallArgs,
> +			Decl,
> +			FullDecl
> +		),	% XXX type_info args should be at the start and
> +			% should be 'in' so that is what this predicate adds
> +			% however, I am not happy with it.

Why?

> +		call_mode_decls_constraints(
> +			ModuleInfo,
> +			VarMap,
> +			PredID,
> +			[FullDecl],
> +			GoalPath,
> +			CallArgs,
> +			Constraints
> +		)	% This pred should strip the disj(conj()) for the
> +			% single declaration.
> +	;	Constraints = []
> +			% XXX Should this be an error?

Yes, a foreign_proc without a mode declaration is an error.

> +	).
> +
> +
> +	% Parallel conjunction
> +	%
> +	% XXX What to do here?
> +	%
> +goal_expr_constraints(_ModuleInfo, _VarMap, _PredID,
> +	par_conj(_Goals), _GoalPath, _Nonlocals, _Constraints) :-
> +	error("build_mode_constraints.m: "
> +		++ "sorry, par_conj not implemented").

Parallel conjuncts must be independent and must produce the same set of
variables, so this will look quite similar to the constraint for a
disjuntion.

Apart from the above comments, this all looke fine to me.  You've done a
good job.

David
-- 
David Overton
WWW: http://www.overtons.id.au/
Mobile Phone (UK): +44 7799 344 322
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------