[m-dev.] for review: last call modulo constructors [2/3]

[Andrew Bromage]

G'day all.

Fergus, I think we should hold a meeting some time soon to go
through inst_matches_*, merge_inst and abstractly_unify_inst in
detail.  We should probably wait until I've finished the instmap
threading through inst_matches_*, since I don't want to write
whole slabs of this stuff again. :-)

David, I think you can commit this onto the alias branch before
we do this meeting, if for no other reason than it can go on
independently with whatever you're doing next..

David Matthew Overton wrote:

> Index: compiler/inst_match.m
> ===================================================================
> RCS file: /home/staff/zs/imp/mercury/compiler/inst_match.m,v
> retrieving revision 1.34.2.10
> diff -u -r1.34.2.10 inst_match.m
> --- 1.34.2.10	1998/03/26 00:36:09
> +++ inst_match.m	1998/06/22 01:02:56

> @@ -310,14 +321,15 @@
>  
>  inst_matches_initial_3(any(UniqA), any(UniqB), _, _, _) :-
>  	unique_matches_initial(UniqA, UniqB).
> -inst_matches_initial_3(any(_), free, _, _, _).
> -inst_matches_initial_3(free, any(_), _, _, _).
> -inst_matches_initial_3(free, free, _, _, _).
> +inst_matches_initial_3(any(_), free(unique), _, _, _).
> +inst_matches_initial_3(free(unique), any(_), _, _, _).
> +inst_matches_initial_3(free(alias), free(alias), _, _, _). % AAA
> +inst_matches_initial_3(free(unique), free(unique), _, _, _). % AAA

Rather than just AAA, could you actually say what the problem is?  For
example:
	% AAA  free(alias) should match_initial free(unique)
Or:
	% AAA  Probably bogus.  Need to look more closely.

> +pred_inst_matches_2(pred_inst_info(PredOrFunc, ArgModesA, Det),
> +		pred_inst_info(PredOrFunc, ArgModesB, Det),
> +		InstTable, ModuleInfo, Expansions) :-
> +	ArgModesA = argument_modes(_, ModesA),
> +	ArgModesB = argument_modes(_, ModesB),
> +	pred_inst_argmodes_matches(ModesA, ModesB, InstTable, ModuleInfo, 
> +		Expansions).
> +

This doesn't work if there is aliasing in the argument modes.  Please
XXX this.

> @@ -687,7 +707,7 @@
>  :- mode inst_matches_binding_3(in, in, in, in, in) is semidet.
>  
>  % Note that `any' is *not* considered to match `any'.
> -inst_matches_binding_3(free, free, _, _, _).
> +inst_matches_binding_3(free(Aliasing), free(Aliasing), _, _, _).

I think free(alias) _does_ match_binding free(unique) (though not
free(alias_many)).  Not sure, though.  We need to have a more in-depth
talk about this.

> @@ -933,7 +1013,7 @@

> -inst_is_unique_2(free, _, _, _).
> +inst_is_unique_2(free(unique), _, _, _).

free(aliased) is also unique.

> @@ -972,7 +1052,7 @@

> -inst_is_mostly_unique_2(free, _, _, _).
> +inst_is_mostly_unique_2(free(unique), _, _, _).

Ditto for mostly_unique.  (However I don't think that free(aliased_many)
will be mostly_unique unless we use trailing.)

> Index: compiler/inst_util.m
> ===================================================================
> RCS file: /home/staff/zs/imp/mercury/compiler/inst_util.m,v
> retrieving revision 1.3.2.12
> diff -u -r1.3.2.12 inst_util.m
> --- 1.3.2.12	1998/03/26 00:36:23
> +++ inst_util.m	1998/06/22 01:02:56

> @@ -273,7 +273,7 @@

> -                        InstA2 = free,
> +                        InstA2 = free(_),

Now that I think about it, this should be:

	( InstA2 = free(_) ; InstA2 = free(_, _) )

> @@ -281,7 +281,7 @@

> -                        InstB2 = free,
> +                        InstB2 = free(_),

Ditto.  Could you please do this change while you're here?  Thanks.

> +:- pred abstractly_unify_bound_inst_arg_with_free(module_info, 
> +	pair(inst, is_live), inst, inst_table, inst_table).
> +:- mode abstractly_unify_bound_inst_arg_with_free(in, in, out, in, out) is det.
> +
> +abstractly_unify_bound_inst_arg_with_free(_ModuleInfo, Inst - dead, Inst,
> +		InstTable, InstTable).
> +
> +abstractly_unify_bound_inst_arg_with_free(ModuleInfo, Inst0 - live, Inst,
> +		InstTable0, InstTable) :-
> +	inst_expand_defined_inst(InstTable0, ModuleInfo, Inst0, Inst1),
> +	( inst_is_ground_or_any(Inst1, InstTable0, ModuleInfo) ->
> +		Inst = Inst1,
> +		InstTable = InstTable0
> +	; inst_is_free(Inst1, InstTable0, ModuleInfo) ->
> +		(
> +			Inst0 = alias(_),
> +			inst_is_free_alias(Inst0, InstTable0, ModuleInfo)
> +		->
> +			Inst = Inst1,
> +			InstTable = InstTable0
> +		;
> +			inst_table_get_inst_key_table(InstTable0, IKT0),
> +			inst_key_table_add(IKT0, free(alias), IK, IKT),
> +			inst_table_set_inst_key_table(InstTable0, IKT,
> +				InstTable),
> +			Inst = alias(IK)
> +		)
> +	;
> +		Inst = Inst0,
> +		InstTable = InstTable0
> +	).

I agree with Fergus on this one: There should be a version which works
on a list of args since it's so common.

> @@ -1134,15 +1171,16 @@

> -make_any_inst(free, IsLive, Uniq0, Real, UI, any(Uniq), det, UI) :-
> +make_any_inst(free(unique), IsLive, Uniq0, Real, UI, any(Uniq), det, UI) :-
>  	unify_uniq(IsLive, Real, det, unique, Uniq0, Uniq).
> -make_any_inst(free(T), IsLive, Uniq, Real, UI,
> +make_any_inst(free(unique, T), IsLive, Uniq, Real, UI,
>  		defined_inst(Any), det, UI) :-
>  	% The following is a round-about way of doing this
>  	%	unify_uniq(IsLive, Real, det, unique, Uniq0, Uniq),
>  	%	Any = typed_any(Uniq, T).
>  	% without the need for a `typed_any' inst.
> -	Any = typed_inst(T, unify_inst(IsLive, free, any(Uniq), Real)).
> +	Any = typed_inst(T, unify_inst(IsLive, free(unique), any(Uniq),
> +		Real)).

How do you make_any_inst(free(aliased))?  Do you need to?

> @@ -1696,7 +1734,8 @@

> -inst_merge_3(free, free, InstTable, M, free, InstTable, M).
> +inst_merge_3(free(Aliasing), free(Aliasing), InstTable, M, free(Aliasing),
> +		InstTable, M).

This is a difficult case.  I think that merging free(unique) and
free(alias) would require extra code.  Best to leave this an error
for now.

> Index: compiler/liveness.m
> ===================================================================
> RCS file: /home/staff/zs/imp/mercury/compiler/liveness.m,v
> retrieving revision 1.81.2.9
> diff -u -r1.81.2.9 liveness.m
> --- 1.81.2.9	1998/06/17 04:13:04
> +++ liveness.m	1998/06/22 01:02:57
> @@ -135,8 +135,8 @@
>  
>  	% Return the set of variables live at the start of the procedure.
>  
> -:- pred initial_liveness(proc_info, pred_id, module_info, set(var)).
> -:- mode initial_liveness(in, in, in, out) is det.
> +:- pred initial_liveness(proc_info, pred_id, module_info, set(var), set(var)).
> +:- mode initial_liveness(in, in, in, out, out) is det.

I think you should alter the documentation of this predicate to explain
what the new argument means.

Part 3 coming soon...

Cheers,
Andrew Bromage