[m-dev.] for review: alias branch improvements [2/2]

[Fergus Henderson]

On 26-Jul-1999, David Overton <dmo at cs.mu.OZ.AU> wrote:
> On Sun, Jul 25, 1999 at 12:28:52PM EST, Fergus Henderson wrote:
> > Is it an invariant that all initial insts of procedures will have no
> > alias substitutions on them, or are you just assuming that this predicate
> > won't be called for procedures whose initial insts have alias substitutions?
> 
> It is an invariant.  It also applies to the final insts from the
> proc_info argument_modes.

OK, that should be documented in hlds_pred.m then.

> > Why do prog_*.m need to import inst_table.m?
> 
> A couple of the types defined in prog_data.m contain inst_tables:
> 
> :- type types_and_modes
> 	--->	types_and_modes(inst_table, list(type_and_mode)).
> 
> :- type argument_modes	--->	argument_modes(inst_table, list(mode)).
> 
> These are types that Andrew added to replace `list(type_and_mode)' and
> `list(mode)' in places where an inst_table is also required for the
> modes to make sense (i.e. most places).  These two types are used in
> several other type definitions in prog_data, e.g. for the `item' type.
> 
> I believe the reason for having inst_tables within the item type is
> that it will be necessary when (and if) we allow users to specify
> aliasing in mode declarations.

OK. 

(It might be better [more consistent with the approach used elsewhere]
to use a less processed representation in the parse tree,
e.g. storing an inst_varset rather than inst_table, but that is not
an important issue.)

>  compiler/instmap.m:
> +	% inst_matches_aliasing(IgnoreAliasing, InstA, InstB,
> +	%		InstMapA, AliasMap0, AliasMap).
> +	% 	If we are not ignoring aliasing, compare the aliasing of two
> +	%	insts with respect to the current alias_map and update the
> +	%	alias_map to include new aliasing information.
> +	%	InstA must be at least as aliased as InstB for the predicate
> +	%	to succeed.

Why is the instmap named InstMapA?  Is it supposed to be related
to InstA rather than InstB?  If so, the documentation doesn't explain this.
If not, then I suggest you rename it as just `InstMap'.

> +++ ./prog_data.m	Mon Jul 26 13:37:07 1999
> +	% Create an other_inst_id_sub to be used for comparing other_inst_ids
> +	% from different procedures.
> +
>  :- func other_inst_id_create_sub(other_inst_id) = other_inst_id_sub.

You should document this one in more detail.
The idea is that the interface documentation should be sufficiently
detailed that a reader should be able to figure out how to use a predicate
without having to look at the definition of that predicate.  This is not
the case here.

E.g. what's the meaning of the argument here?
What are the semantics of the return value
(i.e. what meaningful operations can you perform with it)?

> +	% normalise_unify_id removes all unnecessary aliases and inst_table
> +	% entries from the unify_proc_id.
> +	% Aliases are retained only between corresponding nodes in the
> +	% inst trees of the two arguments.  Such aliases allow us to
> +	% produce more efficient code for the unification procedure
> +	% since, if we know that two corresponding nodes are already
> +	% aliased, we don't need to produce code to unify them.
> +	% (In such cases the unification goal will be replaced by conj([])
> +	% during mode checking by categorize_unify_var_var).

This is done not just for efficiency -- it's also needed for correctness.
It's not just that the generated code would be more efficient,
it's also important that the determinism could be different.

Cheers,
	Fergus.

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