for review: deforestation [4/4]

[Fergus Henderson]

On 19-Feb-1998, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> :- module pd_info.
...
> :- type unfold_info
> 	--->	unfold_info(
> 			proc_info,
> 			instmap,
> 			int,		% cost delta
> 			local_term_info,% local termination info
> 			pred_info,
> 			set(pred_proc_id),
> 			pred_proc_id,	% current pred_proc_id
> 			bool,		% does determinism analysis 
> 					% need to be run
> 			int,		% size delta
> 			unit
> 		).	

A general comment about what `unfold_info' represents would be helpful
here.

What exactly are the cost & size deltas?

> :- interface.
> 
> :- pred pd_info__search_version(hlds_goal::in, maybe_version::out,
> 	pd_info::pd_info_di, pd_info::pd_info_uo) is det.
> 
> :- pred pd_info__define_new_pred(hlds_goal::in, pred_proc_id::out,
> 	hlds_goal::out, pd_info::pd_info_di, pd_info::pd_info_uo) is det.

These two predicates should be documented.

> pd_info__define_new_pred(Goal, PredProcId, CallGoal) -->
> 	pd_info_get_instmap(InstMap),
> 	{ Goal = _ - GoalInfo },
> 	{ goal_info_get_nonlocals(GoalInfo, NonLocals) },
> 	{ set__to_sorted_list(NonLocals, Args) },
> 	pd_info_get_counter(Counter0),
> 	{ Counter is Counter0 + 1 },
> 	pd_info_set_counter(Counter),
> 	{ string__format("_mercury_deforestation__%i", [i(Counter0)], Name) },

Did you modify the name demangler to handle these predicates?

I think `__deforestation__%i' would be better;
the eventual mangled symbol name will already include "mercury_".

> % File: pd_term.m
> % Main author: stayl
> %
> % Termination checking for partial deduction / deforestation.

Why does partial deduction / deforestation need termination checking?

Is this checking the termination of the program being deforested,
or the termination of the deforestation optimization?

> :- import_module string.

That should be at the top of the implementation section,
not in some random spot in the middle of the module.

> % File pd_util.m
...
> 	% inst_msg(InstA, InstB, InstC):
> 	% 	Take the most specific generalisation of two insts.

I suggest that you use the name `inst_MSG', since MSG is an acronym
(this avoids confusion with `msg' as a contraction of `message'.)

... to be continued.

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