[m-rev.] For review: State Variable Transformation

[Simon Taylor]

On 08-Apr-2002, Ralph Becket <rafe at cs.mu.OZ.AU> wrote:
>  :- pred transform_goal_2(goal_expr, prog_context, prog_varset,
>  		prog_substitution, hlds_goal, prog_varset,
> -		transform_info, transform_info, io__state, io__state).
> -:- mode transform_goal_2(in, in, in, in, out, out, in, out, di, uo) is det.
> +		transform_info, transform_info,
> +		dot_colon_subst, dot_colon_subst, in_atom,
> +		io__state, io__state).
> +:- mode transform_goal_2(in, in, in, in, out, out, in, out, in, out, in, di, uo)
> +		is det.

As far as I can tell, in the vast majority of cases the new arguments
are just threaded through. It would be better if they were added to
the transform_info.

> @@ -7218,10 +7462,14 @@
>  		{ Det = Det1 },
>  		{ term__coerce(GoalTerm1, GoalTerm) },
>  		{ parse_goal(GoalTerm, VarSet0, ParsedGoal, VarSet1) },
> +		% Note that all state variables appearing in a lambda
> +		% are considered local to the lambda - this is a stricter
> +		% rule than that applied to ordinary logical variables.
>  		build_lambda_expression(X, PredOrFunc, EvalMethod, Vars1,
>  			Modes, Det, ParsedGoal, VarSet1,
>  			Context, MainContext, SubContext, Goal, VarSet,
> -			Info2, Info)
> +			Info2, Info),
> +		{ DotColonSubst = DotColonSubst0 }

I don't like this rule. It should be possible to refer to !.Var
inside a lambda expression. I'd be happy for references to !:Var
for a state variable non-local to the lambda expression to be
disallowed.

> @@ -7268,16 +7517,26 @@
>  	->
>  		check_expr_purity(Purity, Context, Info0, Info1),
>  		{ map__init(Subst) },
> -		transform_goal(IfParseTree, VarSet11, Subst, IfGoal, VarSet22,
> -			Info1, Info2),
> +
> +		% Since if-then-else expressions can only appear in
> +		% unification goals or as (expressions in) arguments
> +		% to calls, they therefore can only appear in the
> +		% context of an atomic formula.

This comment needs further explanation as to why this is important.

> +	% XXX rafe: we may well have to number the "SVar_(Dot|Colon)_X"s
> +	% apart.  Since the HLDS is dumped and read in for transitive
> +	% intermodule optimization, reusing the same var name won't do.
>
> +:- pred new_svar(prog_var, prog_var, prog_varset, prog_varset).
> +:- mode new_svar(in, out, in, out) is det.
> +
> +new_svar(SVar, Var, VarSet0, VarSet) :-
> +	VarName = "SVAR_" ++ lookup_name(VarSet0, SVar),
> +	varset__new_uniquely_named_var(VarSet0, VarName, Var, VarSet).
> +

Duplicated variable names don't matter for inter-module optimization
because variable numbers are appended to the variables in the clauses
written in `.opt' files.

It would be nice for the variable names to be consecutively numbered
starting at zero like we do for DCGs, but I'm not too fussy about that.

Can I also suggest using something like STATE_Var rather than SVAR_Var.
I hate <letter><word> abbreviations.

> +% XXX rafe: why does the compiler complain about this?
> +% :- pred preceding_in_atom(in_atom, in_atom, dot_colon_subst, dot_colon_subst).
> +% :- mode preceding_in_atom(in, out, in, out) is det.
> +%
> +% preceding_in_atom(DotColonSubst, DotColonSubst,
> +% 		  yes(_),        yes(DotColonSubst ^ dot_subst)).
> +% 
> +% preceding_in_atom(DotColonSubst, new_dot_colon_subst,
> +% 		  no(_),         no(DotColonSubst ^ dot_subst)).

Remove this.

> Index: compiler/prog_io.m
> ===================================================================
> -parse_item(ModuleName, VarSet, Term, Result) :-
> +parse_item(ModuleName, VarSet, Term0, Result) :-
> +	Term = expand_dot_colon_state_vars(Term0),
>   	( %%% some [Decl, DeclContext]
>  		Term = term__functor(term__atom(":-"), [Decl], _DeclContext)
>  	->
> @@ -3415,5 +3416,32 @@
>  
>  :- pred root_module_name(module_name::out) is det.
>  root_module_name(unqualified("")).
> +
> +%------------------------------------------------------------------------------%
> +
> +:- func expand_dot_colon_state_vars(term) = term.
> +
> +expand_dot_colon_state_vars(T @ variable(_)) = T.
> +
> +expand_dot_colon_state_vars(functor(Const, Args, Ctxt)) =
> +	functor(
> +		Const,
> +		list__foldr(expand_dot_colon_state_var, Args, []),
> +		Ctxt
> +	).
> +
> +
> +:- func expand_dot_colon_state_var(term, list(term)) = list(term).
> +
> +expand_dot_colon_state_var(T @ variable(_), Ts) = [T | Ts].
> +
> +expand_dot_colon_state_var(T @ functor(Const, Args, Ctxt), Ts) =
> +	( if Const = atom("!"), Args = [variable(_SVar)] then
> +		[ functor(atom("!."), Args, Ctxt),
> +		  functor(atom("!:"), Args, Ctxt)
> +		| Ts ]
> +	  else
> +	  	[ expand_dot_colon_state_vars(T) | Ts ]
> +	).

Does this really belong here? I think this would be better done in
make_hlds__insert_arg_unifications or make_hlds__make_fresh_arg_vars.
Also, this code will increase the runtime of the compiler by about 5%
-- copying every term read is expensive. This could be improved by
checking at every step whether the sub-terms were changed before
constructing a new term, but I think doing this expansion in make_hlds
is the better solution.

To be continued.

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