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

[Ralph Becket]

Simon Taylor, Friday, 12 April 2002:
> 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.

After having spent some time trying this suggestion out, I'm of the
opinion that this is the wrong thing to do.  The data flow for the
transform_info is quite different to that of the dot_colon_substs and
the in_atom parameter; I've tried merging them together but the
resulting code is messy and obscures what is going on more than does
passing around the extra arguments.  The problem would be less
significant if the transform_info was a DCG thread, but that honour is
already given to the io__state.  (I've discussed this at the keyboard
with Simon and we're in agreement.)

> > @@ -7218,10 +7462,14 @@
> > +		% 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.
> 
> 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.

I've addressed this in a response to your other posting.

> > +		% 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.

The comment has been removed - it was just there as a reminder-to-self
during debugging; a better explanation of the situation is given in the
documentation.

> > +	% 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.

Okay, but they do need to be named to avoid confusion in e.g. type error
messages.  I've elided the XXX.

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

The state variable transformation works right-to-left to avoid
propagating "dead" state variables, which makes it difficult to number
variables in ascending order.

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

No problem - I've changed it to "STATE_VARIABLE_".

> > +% 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.

Done.

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

Okay, I've moved the argument substitution into make_hlds and made it
work on a per-call basis rather than as a whole-term transformation.

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