diff: bug fix for dense_switch.m

[Fergus Henderson]

Hi,

Zoltan and/or Tom, could you please review this?

compiler/dense_switch.m:
	Fix a bug introduced by Zoltan's changes to delay flushing
	of variables needed on backtracking.
	The bug was that the final code_info after a nondet
	dense_switch had not unset the top failure continuation.
	This caused the code generator to generate incorrect C code for
	the code following the dense switch: the generated C code
	clobbered the topmost redoip, rather than using a new temp frame.
	The fix was to ensure that we use the code_info from the
	end of one of the switch branches (for which we will have called
	code_info__unset_failure_cont from code_info__branch_end)
	rather than using the initial code_info and just resetting
	the live variables.

compiler/code_gen.m:
compiler/code_info.m:
compiler/disj_info.m:
compiler/ite_gen.m:
	Various minor improvements to the comments.

tests/general/space.m:
	The regression test for the above-mentioned bug,
	from Tom's original bug report.

cvs diff: Diffing .
Index: code_gen.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/code_gen.m,v
retrieving revision 1.24
diff -u -r1.24 code_gen.m
--- code_gen.m	1997/05/05 11:16:40	1.24
+++ code_gen.m	1997/05/19 05:03:18
@@ -906,7 +906,6 @@
 		Code) -->
 		% This code is a cut-down version of the code for semidet
 		% if-then-elses.
-		% XXX It does not save or restore tickets
 
 	code_info__make_known_failure_cont(ResumeVars, ResumeLocs, no,
 		no, _, ModContCode),
Index: code_info.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/code_info.m,v
retrieving revision 1.199
diff -u -r1.199 code_info.m
--- code_info.m	1997/04/25 09:14:27	1.199
+++ code_info.m	1997/05/19 07:53:44
@@ -1194,8 +1194,8 @@
 			;
 				{ TempFrameCode = node([
 					assign(redoip(lval(maxfr)),
-						const(code_addr_const(RedoAddr)))
-						- "Set failure continuation"
+					    const(code_addr_const(RedoAddr)))
+				- "Set failure continuation on temp frame"
 				]) }
 			),
 			{ HaveTempFrame = yes }
@@ -1348,7 +1348,7 @@
 			ResetCode = node([
 				assign(redoip(lval(maxfr)),
 					const(code_addr_const(RedoAddress)))
-					- "restore failure cont"
+					- "restore known failure cont"
 			])
 		}
 	),
Index: dense_switch.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/dense_switch.m,v
retrieving revision 1.23
diff -u -r1.23 dense_switch.m
--- dense_switch.m	1997/02/19 05:30:00	1.23
+++ dense_switch.m	1997/05/19 09:52:20
@@ -162,17 +162,19 @@
 	),
 		% Now generate the jump table and the cases
 	dense_switch__generate_cases(Cases, StartVal, EndVal, CodeModel,
-			StoreMap, EndLabel, Labels, CasesCode, no, MLiveness),
-		% We keep track of what variables are supposed to be
-		% live at the end of cases. We have to do this explicitly
-		% because generating a `fail' slot last would yield the
-		% wrong liveness.
+			StoreMap, EndLabel, Labels, CasesCode, no,
+			MaybeFinalCodeInfo),
+		% We keep track of the code_info at the end of one of
+		% the non-fail cases.  We have to do this because 
+		% generating a `fail' slot last would yield the
+		% wrong liveness and would not unset the failure cont
+		% for a nondet switch.
 	(
-		{ MLiveness = yes(Liveness) }
+		{ MaybeFinalCodeInfo = yes(FinalCodeInfo) }
 	->
-		code_info__set_forward_live_vars(Liveness)
+		code_info__slap_code_info(FinalCodeInfo)
 	;
-		{ error("dense_switch__generate: no liveness!") }
+		{ error("dense_switch__generate: no final code_info") }
 	),
 	{ DoJump = node([
 		computed_goto(Index, Labels)
@@ -183,22 +185,22 @@
 
 :- pred dense_switch__generate_cases(cases_list, int, int,
 	code_model, store_map, label, list(label), code_tree, 
-	maybe(liveness_info), maybe(liveness_info), code_info, code_info).
+	maybe(code_info), maybe(code_info), code_info, code_info).
 :- mode dense_switch__generate_cases(in, in, in, in, in, in, out, out,
 	in, out, in, out) is det.
 
 dense_switch__generate_cases(Cases0, NextVal, EndVal, CodeModel, StoreMap,
-		EndLabel, Labels, Code, Liveness0, Liveness) -->
+		EndLabel, Labels, Code, MaybeCodeInfo0, MaybeCodeInfo) -->
 	(
 		{ NextVal > EndVal }
 	->
 		{ Code = node([ label(EndLabel) - "End of dense switch" ]) },
 		{ Labels = [] },
-		{ Liveness = Liveness0 }
+		{ MaybeCodeInfo = MaybeCodeInfo0 }
 	;
 		code_info__get_next_label(ThisLabel),
 		dense_switch__generate_case(Cases0, NextVal, CodeModel,
-			StoreMap, Cases1, ThisCode, Comment, NewLiveness),
+			StoreMap, Cases1, ThisCode, Comment, MaybeNewCodeInfo),
 		{ ThisCaseCode = tree(
 			node([ label(ThisLabel) - Comment ]),
 			tree(	ThisCode,
@@ -206,13 +208,13 @@
 					- "branch to end of dense switch" ])
 			)
 		) },
-		{ dense_switch__merge_maybe_liveness(Liveness0, NewLiveness,
-				Liveness1) },
+		{ dense_switch__merge_maybe_code_info(MaybeCodeInfo0,
+				MaybeNewCodeInfo, MaybeCodeInfo1) },
 			% generate the rest of the cases.
 		{ NextVal1 is NextVal + 1 },
 		dense_switch__generate_cases(Cases1, NextVal1, EndVal,
 			CodeModel, StoreMap, EndLabel, Labels1, OtherCasesCode,
-			Liveness1, Liveness),
+			MaybeCodeInfo1, MaybeCodeInfo),
 		{ Labels = [ThisLabel | Labels1] },
 		{ Code = tree(ThisCaseCode, OtherCasesCode) }
 	).
@@ -220,47 +222,47 @@
 %---------------------------------------------------------------------------%
 
 :- pred dense_switch__generate_case(cases_list, int, code_model, store_map,
-		cases_list, code_tree, string,
-			maybe(liveness_info), code_info, code_info).
-:- mode dense_switch__generate_case(in, in, in, in, out, out, out, out, in, out)
-	is det.
+		cases_list, code_tree, string, maybe(code_info),
+		code_info, code_info).
+:- mode dense_switch__generate_case(in, in, in, in, out, out, out, out,
+		in, out) is det.
 
 dense_switch__generate_case(Cases0, NextVal, CodeModel, StoreMap, Cases,
-		Code, Comment, ML) -->
+		Code, Comment, MaybeCodeInfo) -->
 	(
 		{ Cases0 = [Case | Cases1] },
 		{ Case = case(_, int_constant(NextVal), _, Goal) }
 	->
+		{ Comment = "case of dense switch" },
 		% We need to save the expression cache, etc.,
 		% and restore them when we've finished
-		{ Comment = "case of dense switch" },
-		code_info__grab_code_info(CodeInfo),
+		code_info__grab_code_info(CodeInfoAtStart),
 		code_gen__generate_goal(CodeModel, Goal, GoalCode),
 		code_info__generate_branch_end(CodeModel, StoreMap, SaveCode),
 		{ Code = tree(GoalCode, SaveCode) },
-		code_info__get_forward_live_vars(L),
-		code_info__slap_code_info(CodeInfo),
+		code_info__grab_code_info(CodeInfoAtEnd),
+		code_info__slap_code_info(CodeInfoAtStart),
 		{ Cases = Cases1 },
-		{ ML = yes(L) }
+		{ MaybeCodeInfo = yes(CodeInfoAtEnd) }
 	;
 		% This case didn't occur in the original case list - just
 		% generate a `fail' for it.
 		{ Comment = "compiler-introduced `fail' case of dense switch" },
 		code_info__generate_failure(Code),
 		{ Cases = Cases0 },
-		{ ML = no }
+		{ MaybeCodeInfo = no }
 	).
 
-:- pred dense_switch__merge_maybe_liveness(maybe(liveness_info),
-				maybe(liveness_info), maybe(liveness_info)).
-:- mode dense_switch__merge_maybe_liveness(in, in, out) is det.
+:- pred dense_switch__merge_maybe_code_info(maybe(code_info),
+				maybe(code_info), maybe(code_info)).
+:- mode dense_switch__merge_maybe_code_info(in, in, out) is det.
 
-dense_switch__merge_maybe_liveness(L0, L1, L) :-
+dense_switch__merge_maybe_code_info(CodeInfo0, CodeInfo1, CodeInfo) :-
 	(
-		L0 = no
+		CodeInfo0 = no
 	->
-		L = L1
+		CodeInfo = CodeInfo1
 	;
-		L = L0
+		CodeInfo = CodeInfo0
 	).
 
Index: disj_gen.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/disj_gen.m,v
retrieving revision 1.53
diff -u -r1.53 disj_gen.m
--- disj_gen.m	1997/02/23 06:06:07	1.53
+++ disj_gen.m	1997/05/19 08:09:06
@@ -104,7 +104,8 @@
 	% For a det disj, this goal will be det,
 	% and may be followed by other goals.
 	%
-	% XXX We ought not to restore anything in the first disjunct.
+	% XXX For efficiency, we ought not to restore anything in the
+	% first disjunct.
 
 disj_gen__generate_pruned_disjuncts([], _, _, _, _, _, _, _, _) -->
 	{ error("Empty pruned disjunction!") }.
@@ -250,7 +251,7 @@
 		MaybeHpSlot, MaybeTicketSlot, no, GoalsCode),
 
 		% since we don't know which disjunct we have come from
-		% we must set the current failure continuation to unkown.
+		% we must set the current failure continuation to unknown.
 
 	code_info__unset_failure_cont(FlushResumeVarsCode),
 	code_info__remake_with_store_map(StoreMap),
@@ -261,7 +262,8 @@
 
 %---------------------------------------------------------------------------%
 
-	% XXX We ought not to restore anything in the first disjunct.
+	% XXX For efficiency, we ought not to restore anything in the
+	% 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).
@@ -361,7 +363,7 @@
 		code_info__generate_branch_end(model_non, StoreMap, SaveCode),
 
 		{ EndCode = node([
-			label(EndLabel) - "End of pruned disj"
+			label(EndLabel) - "End of nondet disj"
 		]) },
 		{ Code = tree(RestoreHPCode,
 			 tree(RestorePopTicketCode,
Index: ite_gen.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/ite_gen.m,v
retrieving revision 1.41
diff -u -r1.41 ite_gen.m
--- ite_gen.m	1997/02/23 06:06:43	1.41
+++ ite_gen.m	1997/05/19 09:15:46
@@ -212,7 +212,7 @@
 
 	code_info__grab_code_info(CodeInfo),
 
-		% Generate the condition es either a semi-deterministic
+		% Generate the condition as either a semi-deterministic
 		% or as a non-deterministic goal (the failure continuation
 		% must be set up the same way)
 	code_info__push_resume_point_vars(ResumeVars),
@@ -223,6 +223,10 @@
 	( { MaybeMaxfrLval = yes(MaxfrLval) } ->
 		code_info__do_soft_cut(MaxfrLval, SoftCutCode),
 		code_info__unset_failure_cont(FlushCode)
+			% XXX why call unset_failure_cont here?
+			% We're going to call it from branch_end at the
+			% end of the `then' anyway, so is this
+			% one really necessary?
 	;
 		{ SoftCutCode = empty },
 		{ FlushCode = empty }
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.