diff: code_info.m bug fixes

[Fergus Henderson]

Hi,

Zoltan, can you please review this one?

Fix a couple of bugs with nondet code generation.

compiler/code_info.m:
	Fix two bugs in code_info__make_known_failure cont.

	The first bug was that the way it checked for whether a temp
	frame had been created was incorrect; it was only checking
	the HaveTempFrame parameter, which was true only if the most
	recent disjunction had required creation of a temp frame,
	whereas it needs to check the whole failure continuation stack.

	The second bug was that it was doing the wrong thing in the
	case where the current failure continuation was unknown,
	but a temp frame had been created.  It was just clobbering
	the current redoip, whereas what it needs to do is to create
	another temp frame.

compiler/code_gen.m:
compiler/disj_gen.m:
compiler/ite_gen.m:
	Don't pass the HaveTempFrame parameters to and from
	code_info__make_known_failure_cont.

cvs diff: Diffing .
Index: code_gen.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/code_gen.m,v
retrieving revision 1.27
diff -u -r1.27 code_gen.m
--- code_gen.m	1997/05/27 03:06:18	1.27
+++ code_gen.m	1997/06/10 02:44:14
@@ -926,7 +926,7 @@
 		% if-then-elses.
 
 	code_info__make_known_failure_cont(ResumeVars, ResumeLocs, no,
-		no, _, ModContCode),
+		ModContCode),
 
 		% Maybe save the heap state current before the condition;
 		% this ought to be after we make the failure continuation
Index: code_info.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/code_info.m,v
retrieving revision 1.203
diff -c -r1.203 code_info.m
*** code_info.m	1997/05/27 03:06:20	1.203
--- code_info.m	1997/06/10 03:33:46
***************
*** 988,998 ****
  :- pred code_info__manufacture_failure_cont(bool, code_info, code_info).
  :- mode code_info__manufacture_failure_cont(in, in, out) is det.
  
  	% Push a new failure continuation onto the stack.
  
  :- pred code_info__make_known_failure_cont(set(var), resume_locs, bool,
! 	bool, bool, code_tree, code_info, code_info).
! :- mode code_info__make_known_failure_cont(in, in, in, in, out, out, in, out)
  	is det.
  
  	% Generate some code to restore the current redoip, by looking
--- 988,999 ----
  :- pred code_info__manufacture_failure_cont(bool, code_info, code_info).
  :- mode code_info__manufacture_failure_cont(in, in, out) is det.
  
+ 	% make_known_failure_cont(ResumeVars, ResumeLocs, IsNondet, Code):
  	% Push a new failure continuation onto the stack.
  
  :- pred code_info__make_known_failure_cont(set(var), resume_locs, bool,
! 	code_tree, code_info, code_info).
! :- mode code_info__make_known_failure_cont(in, in, in, out, in, out)
  	is det.
  
  	% Generate some code to restore the current redoip, by looking
***************
*** 1123,1128 ****
--- 1124,1146 ----
  code_info__fail_cont_is_unknown(FailContInfo) :-
  	FailContInfo = nondet(unknown, _).
  
+ :- pred code_info__have_temp_frame(fail_stack).
+ :- mode code_info__have_temp_frame(in) is semidet.
+ 
+ 	% have_temp_frame should succeed iff we have created
+ 	% a temp frame on the nondet stack.  It traverses
+ 	% the entire failure continuation stack, looking for
+ 	% any failure continuations with the temp frame maybe(label)
+ 	% set to yes(_).
+ 
+ code_info__have_temp_frame(FailStack0) :-
+ 	stack__pop(FailStack0, FailContInfo, FailStack1),
+ 	(
+ 		FailContInfo = failure_cont(nondet(_, yes(_)), _)
+ 	;
+ 		code_info__have_temp_frame(FailStack1)
+ 	).
+ 
  %---------------------------------------------------------------------------%
  
  code_info__manufacture_failure_cont(IsNondet) -->
***************
*** 1144,1150 ****
  %---------------------------------------------------------------------------%
  
  code_info__make_known_failure_cont(ResumeVars, ResumeLocs, IsNondet,
! 		HaveTempFrame0, HaveTempFrame, ModContCode) -->
  	code_info__get_next_label(OrigLabel),
  	code_info__get_next_label(StackLabel),
  	{ OrigAddr = label(OrigLabel) },
--- 1162,1168 ----
  %---------------------------------------------------------------------------%
  
  code_info__make_known_failure_cont(ResumeVars, ResumeLocs, IsNondet,
! 		ModContCode) -->
  	code_info__get_next_label(OrigLabel),
  	code_info__get_next_label(StackLabel),
  	{ OrigAddr = label(OrigLabel) },
***************
*** 1155,1161 ****
  
  		{ IsNondet = no },
  		{ TempFrameCode = empty },
- 		{ HaveTempFrame = HaveTempFrame0 },
  		{ FailContInfo = semidet }
  	;
  		% In nondet continuations we may use the redoip
--- 1173,1178 ----
***************
*** 1169,1204 ****
  		(
  			{ code_info__fail_cont_is_unknown(OrigInfo) }
  		->
  			code_info__get_next_label(RedoLabel),
  			{ MaybeRedoLabel = yes(RedoLabel) },
  			{ RedoAddr = label(RedoLabel) },
  			(
! 				{ HaveTempFrame0 = no }
  			->
! 					% this code could be better
! 					% (mkframe is a bit of a sledge hammer)
  				{ TempFrameCode = node([
! 					mkframe("temp frame", 1, RedoAddr)
! 						- "create a temporary frame",
! 					assign(curfr, lval(succfr(lval(maxfr))))
! 						- "restore curfr after mkframe"
  				]) }
  			;
  				{ TempFrameCode = node([
  					assign(redoip(lval(maxfr)),
! 					    const(code_addr_const(RedoAddr)))
! 				- "Set failure continuation on temp frame"
  				]) }
! 			),
! 			{ HaveTempFrame = yes }
! 		;
! 			{ MaybeRedoLabel = no },
! 			{ TempFrameCode = node([
! 				assign(redoip(lval(maxfr)),
! 					const(code_addr_const(StackAddr))) -
! 					"Set failure continuation"
! 			]) },
! 			{ HaveTempFrame = HaveTempFrame0 }
  		),
  		{ FailContInfo = nondet(known, MaybeRedoLabel) }
  	),
--- 1186,1240 ----
  		(
  			{ code_info__fail_cont_is_unknown(OrigInfo) }
  		->
+ 			%
+ 			% If the failure continuation is unknown,
+ 			% then we need to create a new temporary frame
+ 			% so that we can make it known
+ 			%
  			code_info__get_next_label(RedoLabel),
  			{ MaybeRedoLabel = yes(RedoLabel) },
  			{ RedoAddr = label(RedoLabel) },
+ 				% this code could be better
+ 				% (mkframe is a bit of a sledge hammer)
+ 			{ TempFrameCode = node([
+ 				mkframe("temp frame", 1, RedoAddr)
+ 					- "create a temporary frame",
+ 				assign(curfr, lval(succfr(lval(maxfr))))
+ 					- "restore curfr after mkframe"
+ 			]) }
+ 		;
+ 			%
+ 			% The failure continuation is known.
+ 			% But did we create a temp frame?
+ 			%
+ 			code_info__get_fail_stack(FailStack),
  			(
! 				{ code_info__have_temp_frame(FailStack) }
  			->
! 				%
! 				% If we created a temp frame, then
! 				% we will need to restore curfr on redo,
! 				% so we set the failure continuation to
! 				% the RedoAddr rather than the usual
! 				% StackAddr.  (generate_failure_cont will
! 				% generate code for it that restores curfr.)
! 				%
! 				code_info__get_next_label(RedoLabel),
! 				{ MaybeRedoLabel = yes(RedoLabel) },
! 				{ RedoAddr = label(RedoLabel) },
  				{ TempFrameCode = node([
! 					assign(redoip(lval(maxfr)),
! 					    const(code_addr_const(RedoAddr)))
! 				    - "Set failure continuation on temp frame"
  				]) }
  			;
+ 				{ MaybeRedoLabel = no },
  				{ TempFrameCode = node([
  					assign(redoip(lval(maxfr)),
! 					    const(code_addr_const(StackAddr)))
! 				    - "Set failure continuation"
  				]) }
! 			)
  		),
  		{ FailContInfo = nondet(known, MaybeRedoLabel) }
  	),
Index: disj_gen.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/disj_gen.m,v
retrieving revision 1.54
diff -u -r1.54 disj_gen.m
--- disj_gen.m	1997/05/20 04:04:26	1.54
+++ disj_gen.m	1997/06/10 03:26:02
@@ -76,7 +76,7 @@
 
 		% Generate all the disjuncts
 	code_info__get_next_label(EndLabel),
-	disj_gen__generate_pruned_disjuncts(Goals, StoreMap, EndLabel, no,
+	disj_gen__generate_pruned_disjuncts(Goals, StoreMap, EndLabel,
 		ReclaimHeap, MaybeHpSlot, MaybeTicketSlot, no, GoalsCode),
 
 		% Remake the code_info using the store map for the
@@ -88,9 +88,9 @@
 %---------------------------------------------------------------------------%
 
 :- pred disj_gen__generate_pruned_disjuncts(list(hlds_goal), store_map,
-	label, bool, bool, maybe(lval), maybe(lval), bool, code_tree,
+	label, bool, maybe(lval), maybe(lval), bool, code_tree,
 	code_info, code_info).
-:- mode disj_gen__generate_pruned_disjuncts(in, in, in, in, in, in, in, in,
+:- mode disj_gen__generate_pruned_disjuncts(in, in, in, in, in, in, in,
 	out, in, out) is det.
 
 	% To generate code for a det or semidet disjunction,
@@ -107,11 +107,10 @@
 	% XXX For efficiency, we ought not to restore anything in the
 	% first disjunct.
 
-disj_gen__generate_pruned_disjuncts([], _, _, _, _, _, _, _, _) -->
+disj_gen__generate_pruned_disjuncts([], _, _, _, _, _, _, _) -->
 	{ error("Empty pruned disjunction!") }.
 disj_gen__generate_pruned_disjuncts([Goal0 | Goals], StoreMap, EndLabel,
-		HaveTempFrame0, ReclaimHeap,
-		MaybeHpSlot0, MaybeTicketSlot, First, Code) -->
+		ReclaimHeap, MaybeHpSlot0, MaybeTicketSlot, First, Code) -->
 	{ Goal0 = GoalExpr0 - GoalInfo0 },
 	{ goal_info_get_code_model(GoalInfo0, CodeModel) },
 	{ goal_info_get_resume_point(GoalInfo0, Resume) },
@@ -123,7 +122,7 @@
 
 		code_info__push_resume_point_vars(ResumeVars),
 		code_info__make_known_failure_cont(ResumeVars, ResumeLocs, no,
-			HaveTempFrame0, HaveTempFrame, ModContCode),
+			ModContCode),
 			% The next line is to enable Goal to pass the
 			% pre_goal_update sanity check
 		{ goal_info_set_resume_point(GoalInfo0, no_resume_point,
@@ -180,8 +179,8 @@
 		code_info__restore_failure_cont(RestoreContCode),
 
 		disj_gen__generate_pruned_disjuncts(Goals, StoreMap, EndLabel,
-			HaveTempFrame, ReclaimHeap,
-			MaybeHpSlot, MaybeTicketSlot, no, RestCode),
+			ReclaimHeap, MaybeHpSlot, MaybeTicketSlot, no,
+			RestCode),
 
 		{ Code = tree(ModContCode, 
 			 tree(RestoreHPCode,
@@ -247,7 +246,7 @@
 	code_info__maybe_save_hp(ReclaimHeap, SaveHeapCode, MaybeHpSlot),
 
 	code_info__get_next_label(EndLabel),
-	disj_gen__generate_non_disjuncts(Goals, StoreMap, EndLabel, no,
+	disj_gen__generate_non_disjuncts(Goals, StoreMap, EndLabel,
 		MaybeHpSlot, MaybeTicketSlot, no, GoalsCode),
 
 		% since we don't know which disjunct we have come from
@@ -266,14 +265,14 @@
 	% first disjunct.
 
 :- pred disj_gen__generate_non_disjuncts(list(hlds_goal), store_map, label,
-	bool, maybe(lval), maybe(lval), bool, code_tree, code_info, code_info).
-:- mode disj_gen__generate_non_disjuncts(in, in, in, in, in, in, in,
+	maybe(lval), maybe(lval), bool, code_tree, code_info, code_info).
+:- mode disj_gen__generate_non_disjuncts(in, in, in, in, in, in,
 	out, in, out) is det.
 
-disj_gen__generate_non_disjuncts([], _, _, _, _, _, _, _) -->
+disj_gen__generate_non_disjuncts([], _, _, _, _, _, _) -->
 	{ error("empty nondet disjunction!") }.
 disj_gen__generate_non_disjuncts([Goal0 | Goals], StoreMap, EndLabel,
-		HaveTempFrame0, MaybeHpSlot, MaybeTicketSlot, First, Code) -->
+		MaybeHpSlot, MaybeTicketSlot, First, Code) -->
 
 	{ Goal0 = GoalExpr0 - GoalInfo0 },
 	{ goal_info_get_resume_point(GoalInfo0, Resume) },
@@ -285,7 +284,7 @@
 
 		code_info__push_resume_point_vars(ResumeVars),
 		code_info__make_known_failure_cont(ResumeVars, ResumeLocs, yes,
-			HaveTempFrame0, HaveTempFrame, ModContCode),
+			ModContCode),
 			% The next line is to enable Goal to pass the
 			% pre_goal_update sanity check
 		{ goal_info_set_resume_point(GoalInfo0, no_resume_point,
@@ -330,8 +329,7 @@
 		code_info__restore_failure_cont(RestoreContCode),
 
 		disj_gen__generate_non_disjuncts(Goals, StoreMap, EndLabel,
-			HaveTempFrame, MaybeHpSlot, MaybeTicketSlot, no,
-			RestCode),
+			MaybeHpSlot, MaybeTicketSlot, no, RestCode),
 
 		{ Code = tree(ModContCode, 
 			 tree(RestoreHPCode,
Index: ite_gen.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/ite_gen.m,v
retrieving revision 1.42
diff -u -r1.42 ite_gen.m
--- ite_gen.m	1997/05/20 04:04:30	1.42
+++ ite_gen.m	1997/06/10 02:47:21
@@ -71,7 +71,7 @@
 		{ error("condition of an if-then-else has no resume point") }
 	),
 	code_info__make_known_failure_cont(ResumeVars, ResumeLocs, no,
-		no, _, ModContCode),
+		ModContCode),
 		% The next line is to enable Cond to pass the
 		% pre_goal_update sanity check
 	{ goal_info_set_resume_point(CondInfo0, no_resume_point, CondInfo) },
@@ -172,7 +172,7 @@
 		{ error("condition of an if-then-else has no resume point") }
 	),
 	code_info__make_known_failure_cont(ResumeVars, ResumeLocs, NondetCond,
-		no, _, ModContCode),
+		ModContCode),
 		% The next line is to enable Cond to pass the
 		% pre_goal_update sanity check
 	{ goal_info_set_resume_point(CondInfo0, no_resume_point, CondInfo) },
cvs diff: Diffing notes

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