[m-dev.] for review: --no-special-preds

[Fergus Henderson]

A general issue: is the `--no-special-preds' option supposed
to be one that you can use different settings of when compiling
different modules which will then be linked into a single
executable?  If so, then there are several places in the code
(in higher_order.m and simplify.m) which do not handle things correctly.
 If not, then the `--no-special-preds' option should be a grade option and
should thus be included in the mangled grade string and handled specially by
the code in the various files listed in runtime/mercury_grade.h.

On 31-Mar-2000, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> comparison/polymorphism.m:
>         Fix a bug: the code for looking up the special preds for user defined
>         types was handling enums as builtins, which they are not.

Well, the code there was handling enums _in the same way as_ builtins.
While it's clear that enums are not builtins, it would be helpful to
explain why they can't be handled the same.

> Index: compiler/higher_order.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/higher_order.m,v
> retrieving revision 1.62
> diff -u -b -r1.62 higher_order.m
> --- compiler/higher_order.m	2000/03/24 10:27:26	1.62
> +++ compiler/higher_order.m	2000/03/30 07:43:30
> @@ -1623,21 +1626,77 @@
>  	),
>  
>  	(
> -		SpecialId = unify,
> +		% Look for unification or comparison applied directly to
> +		% a builtin or atomic type. This needs to be done separately
> +		% from the case for user-defined types, because we want to
> +		% specialize such calls even if we are not generating any
> +		% special preds.
> +
> +		specializeable_special_call(SpecialId, CalledProc),
>  		type_is_atomic(SpecialPredType, ModuleInfo),

Hmm, I don't understand why this case needs to be done separately.
Couldn't the condition of the if-then-else be a disjunction,
e.g.

		( specializeable_special_call(SpecialId, CalledProc),
  		  type_is_atomic(SpecialPredType, ModuleInfo)
		;
		  HaveSpecialPreds = yes
		)

?  
(Quite there is some reason why that won't work, but the
comment there doesn't say why, which makes that comment
more mystifying than enlightening, IMHO.)

> +:- pred specializeable_special_call(special_pred_id::in, proc_id::in)
> +	is semidet.
> +
> +specializeable_special_call(SpecialId, CalledProc) :-
> +	proc_id_to_int(CalledProc, CalledProcInt),
> +	(
> +		SpecialId = unify,
> +		CalledProcInt = 0
> +	;
> +		% compare has four procedures numbered 0 to 3 with identical
> +		% behavior, whose two input arguments' modes are all the
> +		% possible combinations of (ui,in) with (ui,in).
> +		SpecialId = compare,
> +		CalledProcInt =< 3
> +	).

That predicate should probably go in special_pred.m.

> +:- pred generate_unsafe_type_cast(module_info::in, (type)::in,
> +	prog_var::in, prog_var::out, hlds_goal::out,
> +	proc_info::in, proc_info::out) is det.
> +
> +generate_unsafe_type_cast(ModuleInfo, ToType, Arg, CastArg, Goal,
> +		ProcInfo0, ProcInfo) :-
> +	module_info_get_predicate_table(ModuleInfo, PredicateTable),
> +	mercury_private_builtin_module(MercuryBuiltin),
> +	(
> +		predicate_table_search_pred_m_n_a(PredicateTable,
> +			MercuryBuiltin, "unsafe_type_cast", 2, [PredIdPrime])
> +	->
> +		PredId = PredIdPrime
> +	;
> +		error("generate_unsafe_type_cast: pred table lookup failed")
> +	),
> +	proc_id_to_int(ProcId, 0),
> +	proc_info_create_var_from_type(ProcInfo0, ToType, CastArg, ProcInfo),
> +	set__list_to_set([Arg, CastArg], NonLocals),
> +	instmap_delta_from_assoc_list([CastArg - ground(shared, no)],
> +		InstMapDelta),
> +	goal_info_init(NonLocals, InstMapDelta, det, GoalInfo),
> +	Goal = call(PredId, ProcId, [Arg, CastArg], not_builtin,
> +		no, qualified(MercuryBuiltin, "unsafe_type_cast")) - GoalInfo.

It might be better to use `hlds_pred__initial_proc_id(ProcId)'
rather than `proc_id_to_int(ProcId, 0)'.

Some of this code is quite similar to unify_proc__build_call;
it would be worth having a glance at that and seeing if you
could reuse that code, if you haven't already done so.

simplify.m:
> @@ -1188,45 +1166,100 @@
>  		simplify__goal_2(Call0, GoalInfo0, Call1, GoalInfo),
>  		{ Call = Call1 - GoalInfo },
>  		{ ExtraGoals = [] }
> -
> -	; { type_to_type_id(Type, TypeId, TypeArgs) } ->
> +	;
> +		{ type_to_type_id(Type, TypeIdPrime, TypeArgsPrime) ->
> +			TypeId = TypeIdPrime,
> +			TypeArgs = TypeArgsPrime
> +		;
> +			error("simplify: type_to_type_id failed")
> +		},
> +		{ determinism_components(Det, CanFail, at_most_one) },
> +		{ unify_proc__lookup_mode_num(ModuleInfo, TypeId, UniMode, Det,
> +			ProcId) },
> +		{ module_info_globals(ModuleInfo, Globals) },
> +		{ globals__lookup_bool_option(Globals, special_preds,
> +			SpecialPreds) },
> +		(
> +			{ SpecialPreds = no },
> +			{ proc_id_to_int(ProcId, ProcIdInt) },
> +			{ ProcIdInt = 0 }
> +		->

This is one of the places where you are assuming that
the setting of the special_preds option in one module
implies something about the existence of special predicates
in a different module.

Also the test for the magic number zero here should be replaced by
a call to `hlds_pred__in_in_unification_proc_id(ProcId)'.

> +++ tests/hard_coded/user_defined_equality.m	2000/03/31 01:36:28
...
> @@ -18,7 +29,39 @@
>  
>  main -->
>  	( { append([bar], [baz], [baz, bar]) } ->
> -		print("yes"), nl
> +		io__write_string("yes\n")
>  	;
> -		print("no"), nl
> +		io__write_string("no\n")
> +	),

Any particular reason for that change (print, nl ==> io__write_string)?
In general it is better to leave regression tests unchanged where possible.

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