[m-rev.] diff: fix deep profiling bug

[Fergus Henderson]

Estimated hours taken: 16
Branches: main, release

Fix a bug that caused an internal error in the Mercury compiler when
compiling tests/dppd/transpose in grade asm_fast.gc.profdeep 
with -O3 --intermodule-optimization.

compiler/simplify.m:
	When merging nested somes into a single some,
	preserve the `keep_this_commit' goal feature
	if it is present on any of the nested somes.

compiler/hlds_goal.m:
	Move the documentation of the `can_remove' field in
	quantifications to next to the definition of the `can_remove'
	type, and expand on it a little.

Workspace: /home/ceres/fjh/mercury
Index: compiler/hlds_goal.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/hlds_goal.m,v
retrieving revision 1.99
diff -u -d -r1.99 hlds_goal.m
--- compiler/hlds_goal.m	9 Sep 2002 07:48:12 -0000	1.99
+++ compiler/hlds_goal.m	3 Feb 2003 06:35:43 -0000
@@ -131,13 +131,8 @@
 		% (except to recompute the goal_info quantification).
 		% `all Vs' gets converted to `not some Vs not'.
 		% The second argument is `can_remove' if the quantification
-		% is allowed to be removed. A non-removable explicit
-		% quantification may be introduced to keep related goals
-		% together where optimizations that separate the goals
-		% can only result in worse behaviour. An example is the
-		% closures for the builtin aditi update predicates -
-		% they should be kept close to the update call where
-		% possible to make it easier to use indexes for the update.
+		% is allowed to be removed; see the docs for the can_remove
+		% type.
 	;	{ some(
 			some_exist_vars	:: list(prog_var),
 			some_can_remove	:: can_remove,
@@ -250,6 +245,19 @@
 % Information for quantifications
 %
 
+	% The second argument of explicit quantification goals
+	% is `can_remove' if the quantification is allowed to
+	% be removed.  A non-removable explicit
+	% quantification may be introduced to keep related goals
+	% together where optimizations that separate the goals
+	% can only result in worse behaviour. An example is the
+	% closures for the builtin aditi update predicates -
+	% they should be kept close to the update call where
+	% possible to make it easier to use indexes for the update.
+	%
+	% See also the closely related `keep_this_commit' goal_feature.
+	% XXX Why do we have both cannot_remove and keep_this_commit?
+	%     Do we really need both?
 :- type can_remove
 	--->	can_remove
 	;	cannot_remove.
Index: compiler/simplify.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/simplify.m,v
retrieving revision 1.106
diff -u -d -r1.106 simplify.m
--- compiler/simplify.m	1 Nov 2002 07:06:57 -0000	1.106
+++ compiler/simplify.m	3 Feb 2003 06:37:53 -0000
@@ -1621,8 +1621,8 @@
 		hlds_goal::in, hlds_goal_info::in, hlds_goal::out) is det.
 
 simplify__nested_somes(CanRemove0, Vars1, Goal0, OrigGoalInfo, Goal) :-
-	simplify__nested_somes_2(CanRemove0, Vars1, Goal0,
-		CanRemove, Vars, Goal1),
+	simplify__nested_somes_2(CanRemove0, no, Vars1, Goal0,
+		CanRemove, KeepThisCommit, Vars, Goal1),
 	Goal1 = GoalExpr1 - GoalInfo1,
 	(
 		goal_info_get_determinism(GoalInfo1, Detism),
@@ -1633,29 +1633,43 @@
 		% is unnecessary.
 		Goal = GoalExpr1 - GoalInfo1
 	;
-		Goal = some(Vars, CanRemove, Goal1) - OrigGoalInfo
+		% The `some' needs to be kept.
+		% However, we may still have merged multiple nested somes
+		% into a single `some'.  This is OK, but we need to be careful
+		% to ensure that we don't lose the `keep_this_commit' flag
+		% (if any) on the nested somes.
+		( KeepThisCommit = yes ->
+			goal_info_add_feature(OrigGoalInfo, keep_this_commit,
+				GoalInfo)
+		;
+			GoalInfo = OrigGoalInfo
+		),
+		Goal = some(Vars, CanRemove, Goal1) - GoalInfo
 	).
 
-:- pred simplify__nested_somes_2(can_remove::in, list(prog_var)::in,
-		hlds_goal::in, can_remove::out, list(prog_var)::out,
+:- pred simplify__nested_somes_2(can_remove::in, bool::in, list(prog_var)::in,
+		hlds_goal::in, can_remove::out, bool::out, list(prog_var)::out,
 		hlds_goal::out) is det.
 
-simplify__nested_somes_2(CanRemove0, Vars0, Goal0, CanRemove, Vars, Goal) :-
-	( Goal0 = some(Vars1, CanRemove1, Goal1) - _ ->
-		(
-			( CanRemove0 = cannot_remove
-			; CanRemove1 = cannot_remove
-			)
-		->
+simplify__nested_somes_2(CanRemove0, KeepThisCommit0, Vars0, Goal0,
+		CanRemove, KeepThisCommit, Vars, Goal) :-
+	( Goal0 = some(Vars1, CanRemove1, Goal1) - GoalInfo0 ->
+		( goal_info_has_feature(GoalInfo0, keep_this_commit) ->
+			KeepThisCommit2 = yes
+		;
+			KeepThisCommit2 = KeepThisCommit0
+		),
+		( CanRemove1 = cannot_remove ->
 			CanRemove2 = cannot_remove
 		;
-			CanRemove2 = can_remove
+			CanRemove2 = CanRemove0
 		),
 		list__append(Vars0, Vars1, Vars2),
-		simplify__nested_somes_2(CanRemove2, Vars2, Goal1,
-			CanRemove, Vars, Goal)
+		simplify__nested_somes_2(CanRemove2, KeepThisCommit2, Vars2,
+			Goal1, CanRemove, KeepThisCommit, Vars, Goal)
 	;
 		CanRemove = CanRemove0,
+		KeepThisCommit = KeepThisCommit0,
 		Vars = Vars0,
 		Goal = Goal0
 	).

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