[m-rev.] for review: heap reclamation on failure for MLDS back-end

Fergus Henderson fjh at cs.mu.OZ.AU
Mon Nov 26 18:46:53 AEDT 2001


On 23-Nov-2001, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> On 22-Nov-2001, Peter Ross <peter.ross at miscrit.be> wrote:
> > Inside the then part I think it would be better to throw an exception.
> 
> That's probably a good idea.
> Shall do.

I changed it to call private_builtin__unused, which throws an exception.

> > > +% XXX copied from table_gen.m
> > > +
> > > +:- pred generate_call(string::in, list(prog_var)::in, determinism::in,
> > > +	maybe(goal_feature)::in, assoc_list(prog_var, inst)::in,
> > > +	module_info::in, term__context::in, hlds_goal::out) is det.
> >
> > I would suggest putting this into a new module then.
> 
> I wondered how long I was going to get away with that one ;-)
> I'll find somewhere appropriate to put it.

I chose goal_util.m.

Here's the relative diff for these changes.
I'll go ahead and commit this now.

diff -u backup/CHANGES16 ./CHANGES16
--- backup/CHANGES16	Mon Nov 26 18:37:23 2001
+++ ./CHANGES16	Mon Nov 26 18:43:29 2001
@@ -17,3 +17,17 @@
 compiler/notes/compiler_design.html:
 	Mention the new pass.
 
+compiler/add_trail_ops.m:
+compiler/table_gen.m:
+compiler/goal_util.m:
+	Abstract out the common code from `generate_call' in table_gen.m,
+	add_trail_ops.m, and add_heap_ops.m, and put it in a new procedure
+	`generate_simple_goal' in goal_util.m.
+
+compiler/add_heap_ops.m:
+compiler/add_trail_ops.m:
+	Apply a review suggestion from Peter Ross: when putting code
+	in places that should not be reachable, insert code that calls
+	private_builtin__unused (which calls error/1) rather
+	than just inserting `true'.
+
diff -u backup/add_heap_ops.m ./add_heap_ops.m
--- backup/add_heap_ops.m	Mon Nov 26 17:24:28 2001
+++ ./add_heap_ops.m	Mon Nov 26 18:35:40 2001
@@ -38,7 +38,8 @@
 :- implementation.
 
 :- import_module prog_data, prog_util, (inst).
-:- import_module hlds_goal, hlds_data, quantification, modules, type_util.
+:- import_module hlds_goal, hlds_data.
+:- import_module goal_util, quantification, modules, type_util.
 :- import_module instmap.
 
 :- import_module bool, string.
@@ -128,15 +129,19 @@
 	{ true_goal(Context, True) },
 	{ fail_goal(Context, Fail) },
 	{ map__init(SM) },
+	ModuleInfo =^ module_info,
 	{ NumSolns = at_most_zero ->
 		% The "then" part of the if-then-else will be unreachable,
 		% but to preserve the invariants that the MLDS back-end
 		% relies on, we need to make sure that it can't fail.
-		% So we use `true' rather than `fail' for the "then" part.
-		NewOuterGoal = if_then_else([], InnerGoal, True, True, SM)
+		% So we use a call to `private_builtin__unused' (which
+		% will call error/1) rather than `fail' for the "then" part.
+		generate_call("unused", [], det, no, [], ModuleInfo, Context,
+			ThenGoal)
 	;
-		NewOuterGoal = if_then_else([], InnerGoal, Fail, True, SM)
+		ThenGoal = Fail
 	},
+	{ NewOuterGoal = if_then_else([], InnerGoal, ThenGoal, True, SM) },
 	goal_expr_add_heap_ops(NewOuterGoal, OuterGoalInfo, Goal).
 
 goal_expr_add_heap_ops(some(A, B, Goal0), GoalInfo,
@@ -187,7 +192,7 @@
 		ModuleInfo =^ module_info,
 		{ goal_info_get_context(GoalInfo, Context) },
 		{ generate_call("reclaim_heap_nondet_pragma_foreign_code",
-			[], det, no, [], ModuleInfo, Context,
+			[], erroneous, no, [], ModuleInfo, Context,
 			SorryNotImplementedCode) },
 		{ Goal = SorryNotImplementedCode }
 	;
@@ -294,62 +299,14 @@
 
 %-----------------------------------------------------------------------------%
 
-% XXX copied from table_gen.m
-
 :- pred generate_call(string::in, list(prog_var)::in, determinism::in,
 	maybe(goal_feature)::in, assoc_list(prog_var, inst)::in,
 	module_info::in, term__context::in, hlds_goal::out) is det.
 
 generate_call(PredName, Args, Detism, MaybeFeature, InstMap, Module, Context,
 		CallGoal) :-
-	list__length(Args, Arity),
 	mercury_private_builtin_module(BuiltinModule),
-	module_info_get_predicate_table(Module, PredTable),
-	(
-		predicate_table_search_pred_m_n_a(PredTable,
-			BuiltinModule, PredName, Arity,
-			[PredId0])
-	->
-		PredId = PredId0
-	;
-		string__int_to_string(Arity, ArityS),
-		string__append_list(["can't locate ", PredName,
-			"/", ArityS], ErrorMessage),
-		error(ErrorMessage)
-	),
-	module_info_pred_info(Module, PredId, PredInfo),
-	(
-		pred_info_procids(PredInfo, [ProcId0])
-	->
-		ProcId = ProcId0
-	;
-		string__int_to_string(Arity, ArityS),
-		string__append_list(["too many modes for pred ",
-			PredName, "/", ArityS], ErrorMessage),
-		error(ErrorMessage)
-
-	),
-	Call = call(PredId, ProcId, Args, not_builtin, no,
-		qualified(BuiltinModule, PredName)),
-	set__init(NonLocals0),
-	set__insert_list(NonLocals0, Args, NonLocals),
-	determinism_components(Detism, _CanFail, NumSolns),
-	(
-		NumSolns = at_most_zero
-	->
-		instmap_delta_init_unreachable(InstMapDelta)
-	;
-		instmap_delta_from_assoc_list(InstMap, InstMapDelta)
-	),
-	goal_info_init(NonLocals, InstMapDelta, Detism, CallGoalInfo0),
-	goal_info_set_context(CallGoalInfo0, Context, CallGoalInfo1),
-	(
-		MaybeFeature = yes(Feature),
-		goal_info_add_feature(CallGoalInfo1, Feature, CallGoalInfo)
-	;
-		MaybeFeature = no,
-		CallGoalInfo = CallGoalInfo1
-	),
-	CallGoal = Call - CallGoalInfo.
+	goal_util__generate_simple_call(BuiltinModule, PredName, Args, Detism,
+		MaybeFeature, InstMap, Module, Context, CallGoal).
 
 %-----------------------------------------------------------------------------%
diff -u backup/add_trail_ops.m ./add_trail_ops.m
--- backup/add_trail_ops.m	Mon Nov 26 17:24:28 2001
+++ ./add_trail_ops.m	Mon Nov 26 18:37:59 2001
@@ -19,9 +19,11 @@
 % See compiler/notes/trailing.html for more information about trailing
 % in the Mercury implementation.
 %
+% This pass is very similar to add_heap_ops.m.
+%
 %-----------------------------------------------------------------------------%
 
-% XXX FIXME check goal_infos for correctness
+% XXX check goal_infos for correctness
 
 %-----------------------------------------------------------------------------%
 
@@ -36,7 +38,8 @@
 :- implementation.
 
 :- import_module prog_data, prog_util, (inst).
-:- import_module hlds_goal, hlds_data, quantification, modules, type_util.
+:- import_module hlds_goal, hlds_data.
+:- import_module goal_util, quantification, modules, type_util.
 :- import_module code_model, instmap.
 
 :- import_module bool, string.
@@ -128,15 +131,20 @@
 	{ true_goal(Context, True) },
 	{ fail_goal(Context, Fail) },
 	{ map__init(SM) },
+	ModuleInfo =^ module_info,
 	{ NumSolns = at_most_zero ->
 		% The "then" part of the if-then-else will be unreachable,
 		% but to preserve the invariants that the MLDS back-end
 		% relies on, we need to make sure that it can't fail.
-		% So we use `true' rather than `fail' for the "then" part.
-		NewOuterGoal = if_then_else([], InnerGoal, True, True, SM)
+		% So we use a call to `private_builtin__unused' (which
+		% will call error/1) rather than `fail' for the "then" part.
+		mercury_private_builtin_module(PrivateBuiltin),
+		generate_simple_call(PrivateBuiltin, "unused",
+			[], det, no, [], ModuleInfo, Context, ThenGoal)
 	;
-		NewOuterGoal = if_then_else([], InnerGoal, Fail, True, SM)
+		ThenGoal = Fail
 	},
+	{ NewOuterGoal = if_then_else([], InnerGoal, ThenGoal, True, SM) },
 	goal_expr_add_trail_ops(NewOuterGoal, OuterGoalInfo, Goal).
 
 goal_expr_add_trail_ops(some(A, B, Goal0), OuterGoalInfo,
@@ -264,7 +272,7 @@
 		ModuleInfo =^ module_info,
 		{ goal_info_get_context(GoalInfo, Context) },
 		{ generate_call("trailed_nondet_pragma_foreign_code",
-			[], det, no, [], ModuleInfo, Context,
+			[], erroneous, no, [], ModuleInfo, Context,
 			SorryNotImplementedCode) },
 		{ Goal = SorryNotImplementedCode }
 	;
@@ -455,62 +463,14 @@
 
 %-----------------------------------------------------------------------------%
 
-% XXX copied from table_gen.m
-
 :- pred generate_call(string::in, list(prog_var)::in, determinism::in,
 	maybe(goal_feature)::in, assoc_list(prog_var, inst)::in,
 	module_info::in, term__context::in, hlds_goal::out) is det.
 
 generate_call(PredName, Args, Detism, MaybeFeature, InstMap, Module, Context,
 		CallGoal) :-
-	list__length(Args, Arity),
 	mercury_private_builtin_module(BuiltinModule),
-	module_info_get_predicate_table(Module, PredTable),
-	(
-		predicate_table_search_pred_m_n_a(PredTable,
-			BuiltinModule, PredName, Arity,
-			[PredId0])
-	->
-		PredId = PredId0
-	;
-		string__int_to_string(Arity, ArityS),
-		string__append_list(["can't locate ", PredName,
-			"/", ArityS], ErrorMessage),
-		error(ErrorMessage)
-	),
-	module_info_pred_info(Module, PredId, PredInfo),
-	(
-		pred_info_procids(PredInfo, [ProcId0])
-	->
-		ProcId = ProcId0
-	;
-		string__int_to_string(Arity, ArityS),
-		string__append_list(["too many modes for pred ",
-			PredName, "/", ArityS], ErrorMessage),
-		error(ErrorMessage)
-
-	),
-	Call = call(PredId, ProcId, Args, not_builtin, no,
-		qualified(BuiltinModule, PredName)),
-	set__init(NonLocals0),
-	set__insert_list(NonLocals0, Args, NonLocals),
-	determinism_components(Detism, _CanFail, NumSolns),
-	(
-		NumSolns = at_most_zero
-	->
-		instmap_delta_init_unreachable(InstMapDelta)
-	;
-		instmap_delta_from_assoc_list(InstMap, InstMapDelta)
-	),
-	goal_info_init(NonLocals, InstMapDelta, Detism, CallGoalInfo0),
-	goal_info_set_context(CallGoalInfo0, Context, CallGoalInfo1),
-	(
-		MaybeFeature = yes(Feature),
-		goal_info_add_feature(CallGoalInfo1, Feature, CallGoalInfo)
-	;
-		MaybeFeature = no,
-		CallGoalInfo = CallGoalInfo1
-	),
-	CallGoal = Call - CallGoalInfo.
+	goal_util__generate_simple_call(BuiltinModule, PredName, Args, Detism,
+		MaybeFeature, InstMap, Module, Context, CallGoal).
 
 %-----------------------------------------------------------------------------%
diff -u backup/goal_util.m ./goal_util.m
--- backup/goal_util.m	Mon Nov 26 17:35:51 2001
+++ ./goal_util.m	Mon Nov 26 18:22:36 2001
@@ -6,29 +6,21 @@
 
 % Main author: conway.
 %
-% This module provides some functionality for renaming variables in goals.
-% The predicates rename_var* take a structure and a mapping from var -> var
-% and apply that translation. If a var in the input structure does not
-% occur as a key in the mapping, then the variable is left unsubstituted.
-
-% goal_util__create_variables takes a list of variables, a varset an
-% initial translation mapping and an initial mapping from variable to
-% type, and creates new instances of each of the source variables in the
-% translation mapping, adding the new variable to the type mapping and
-% updating the varset. The type for each new variable is found by looking
-% in the type map given in the 5th argument - the last input.
-% (This interface will not easily admit uniqueness in the type map for this
-% reason - such is the sacrifice for generality.)
-
-:- module goal_util.
+% This module provides various utility procedures for manipulating HLDS goals,
+% e.g. some functionality for renaming variables in goals.
 
 %-----------------------------------------------------------------------------%
 
+:- module goal_util.
 :- interface.
 
 :- import_module hlds_data, hlds_goal, hlds_module, hlds_pred.
-:- import_module instmap, prog_data.
-:- import_module bool, list, set, map, term.
+:- import_module (inst), instmap, prog_data.
+:- import_module assoc_list, bool, list, set, map, term, std_util.
+
+% The predicates rename_var* take a structure and a mapping from var -> var
+% and apply that translation. If a var in the input structure does not
+% occur as a key in the mapping, then the variable is left unsubstituted.
 
 	% goal_util__rename_vars_in_goals(GoalList, MustRename, Substitution,
 	%	NewGoalList).
@@ -48,6 +40,15 @@
 		map(var(T), var(T)), list(var(T))).
 :- mode goal_util__rename_var_list(in, in, in, out) is det.
 
+% goal_util__create_variables takes a list of variables, a varset an
+% initial translation mapping and an initial mapping from variable to
+% type, and creates new instances of each of the source variables in the
+% translation mapping, adding the new variable to the type mapping and
+% updating the varset. The type for each new variable is found by looking
+% in the type map given in the 5th argument - the last input.
+% (This interface will not easily admit uniqueness in the type map for this
+% reason - such is the sacrifice for generality.)
+
 	% goal_util__create_variables(OldVariables, OldVarset, InitialVarTypes,
 	%	InitialSubstitution, OldVarTypes, OldVarNames,  NewVarset,
 	%	NewVarTypes, NewSubstitution)
@@ -198,6 +199,22 @@
 :- pred goal_util__reordering_maintains_termination(module_info::in, bool::in, 
 		hlds_goal::in, hlds_goal::in) is semidet.
 
+	% generate_simple_call(ModuleName, PredName, Args,
+	%		Detism, MaybeFeature, InstMapDelta,
+	%		ModuleInfo, Context, CallGoal):
+	% Generate a call to a builtin procedure (e.g.
+	% from the private_builtin or table_builtin module).
+	% This is used by HLDS->HLDS transformation passes that introduce
+	% calls to builtin procedures.  This is restricted in various ways,
+	% e.g. the called procedure must have exactly one mode,
+	% and at most one type parameter.  So it should only be used
+	% for generating calls to known builtin procedures.
+	%
+:- pred goal_util__generate_simple_call(module_name::in, string::in,
+		list(prog_var)::in, determinism::in, maybe(goal_feature)::in,
+		assoc_list(prog_var, inst)::in, module_info::in,
+		term__context::in, hlds_goal::out) is det.
+
 %-----------------------------------------------------------------------------%
 %-----------------------------------------------------------------------------%
 
@@ -205,7 +222,8 @@
 
 :- import_module hlds_data, mode_util, code_aux, prog_data, purity.
 :- import_module code_aux, det_analysis, inst_match, type_util, (inst).
-:- import_module int, std_util, string, assoc_list, require, varset.
+
+:- import_module int, string, require, varset.
 
 %-----------------------------------------------------------------------------%
 
@@ -1202,5 +1220,68 @@
 	goal_info_get_nonlocals(LaterGoalInfo, LaterGoalNonLocals),
 	set__intersect(EarlierChangedVars, LaterGoalNonLocals, Intersection),
 	not set__empty(Intersection).
+
+%-----------------------------------------------------------------------------%
+
+goal_util__generate_simple_call(ModuleName, PredName, Args, Detism,
+		MaybeFeature, InstMap, Module, Context, CallGoal) :-
+	list__length(Args, Arity),
+	module_info_get_predicate_table(Module, PredTable),
+	(
+		predicate_table_search_pred_m_n_a(PredTable,
+			ModuleName, PredName, Arity,
+			[PredId0])
+	->
+		PredId = PredId0
+	;
+		% Some of the table builtins are polymorphic,
+		% and for them we need to subtract one from the arity
+		% to take into account the type_info argument.
+		predicate_table_search_pred_m_n_a(PredTable,
+			ModuleName, PredName, Arity - 1,
+			[PredId0])
+	->
+		PredId = PredId0
+	;
+		string__int_to_string(Arity, ArityS),
+		string__append_list(["can't locate ", PredName,
+			"/", ArityS], ErrorMessage),
+		error(ErrorMessage)
+	),
+	module_info_pred_info(Module, PredId, PredInfo),
+	(
+		pred_info_procids(PredInfo, [ProcId0])
+	->
+		ProcId = ProcId0
+	;
+		string__int_to_string(Arity, ArityS),
+		string__append_list(["too many modes for pred ",
+			PredName, "/", ArityS], ErrorMessage),
+		error(ErrorMessage)
+
+	),
+	Call = call(PredId, ProcId, Args, not_builtin, no,
+		qualified(ModuleName, PredName)),
+	set__init(NonLocals0),
+	set__insert_list(NonLocals0, Args, NonLocals),
+	determinism_components(Detism, _CanFail, NumSolns),
+	(
+		NumSolns = at_most_zero
+	->
+		instmap_delta_init_unreachable(InstMapDelta)
+	;
+		instmap_delta_from_assoc_list(InstMap, InstMapDelta)
+	),
+	goal_info_init(NonLocals, InstMapDelta, Detism,
+		CallGoalInfo0),
+	goal_info_set_context(CallGoalInfo0, Context, CallGoalInfo1),
+	(
+		MaybeFeature = yes(Feature),
+		goal_info_add_feature(CallGoalInfo1, Feature, CallGoalInfo)
+	;
+		MaybeFeature = no,
+		CallGoalInfo = CallGoalInfo1
+	),
+	CallGoal = Call - CallGoalInfo.
 
 %-----------------------------------------------------------------------------%

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------



More information about the reviews mailing list