[m-rev.] For post-commit review: Loop control scope reason.

[Zoltan Somogyi]

On 13-Sep-2011, Paul Bone <pbone at csse.unimelb.edu.au> wrote:
> This will be easier to review once it is commited,

Post-commit review of Paul's change introducing the loop_control scope reason.

compiler/simplify.m:
	Fix a bug in the handling of the new scope kind.

compiler/make_hlds_warn.m:
compiler/modecheck_goal.m:
	Replace tentative documentation with firm documentation.

compiler/saved_vars.m:
	Improve layout.

Zoltan.

cvs diff: Diffing .
Index: make_hlds_warn.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/make_hlds_warn.m,v
retrieving revision 1.42
diff -u -b -r1.42 make_hlds_warn.m
--- make_hlds_warn.m	13 Sep 2011 06:07:14 -0000	1.42
+++ make_hlds_warn.m	13 Sep 2011 06:28:10 -0000
@@ -253,19 +253,8 @@
                 QuantVars, !Info)
         ;
             Reason = loop_control(_, _),
-            % XXX I understand what this code should do but I don't
-            % understand how it does it.  Therefore I don't know how to
-            % implement this branch.
-            %
-            % The variables used for loop control (LC and LCS) will not be
-            % mentioned by the code within this scope (the call that
-            % eventually uses them is introduced during HLDS->LLDS
-            % translation.
-            %
-            % However, the LCS will be incorrectly considered a singleton
-            % variable because the compiler cannot see its use within this
-            % scope.  Therefore I have to do something to tell the compiler
-            % that it is not a singleton (it has been seen a second time).
+            % These scopes are introduced only by compiler passes
+            % that execute after us.
             sorry($module, $pred, "loop_control")
         )
     ;
Index: modecheck_goal.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/modecheck_goal.m,v
retrieving revision 1.13
diff -u -b -r1.13 modecheck_goal.m
--- modecheck_goal.m	13 Sep 2011 06:07:14 -0000	1.13
+++ modecheck_goal.m	13 Sep 2011 06:30:37 -0000
@@ -812,12 +812,9 @@
             )
         ;
             Reason = loop_control(LCVar, LCSVar),
-            % Here I can double-check that the variables for the loop
-            % control are ground.  XXX Is this the right thing to do?
+            % We check that the variables for the loop control are ground.
             mode_info_get_instmap(!.ModeInfo, InstMap),
             mode_info_get_module_info(!.ModeInfo, ModuleInfo),
-            % Note that these errors are thrown for unreachable code since
-            % the loop control scope must be det or cc_multi.
             expect(var_is_ground_in_instmap(ModuleInfo, InstMap, LCVar),
                 $module, $pred, "Loop control variable is not ground"),
             expect(var_is_ground_in_instmap(ModuleInfo, InstMap, LCSVar),
Index: saved_vars.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/saved_vars.m,v
retrieving revision 1.94
diff -u -b -r1.94 saved_vars.m
--- saved_vars.m	13 Sep 2011 06:07:14 -0000	1.94
+++ saved_vars.m	13 Sep 2011 06:38:34 -0000
@@ -343,18 +343,16 @@
         CanPush = yes
     ).
 
-    % The main inputs of this predicate are a list of goals in a
-    % conjunction, and a goal Construct that assigns a constant to a
-    % variable Var.
-    %
-    % When we find an atomic goal in the conjunction that refers to
-    % Var, we create a new variable NewVar, rename both this goal
-    % and Construct to refer to NewVar instead of Var, and insert
-    % the new version of Construct before the new version of the
-    % goal.
+    % The main inputs of this predicate are a list of goals in a conjunction,
+    % and a goal Construct that assigns a constant to a variable Var.
     %
-    % When we find a non-atomic goal in the conjunction that refers
-    % to Var, we push Construct into each of its components.
+    % When we find an atomic goal in the conjunction that refers to Var,
+    % we create a new variable NewVar, rename both this goal and Construct
+    % to refer to NewVar instead of Var, and insert the new version
+    % of Construct before the new version of the goal.
+    %
+    % When we find a non-atomic goal in the conjunction that refers to Var,
+    % we push Construct into each of its components.
     %
     % If Var is exported from the conjunction, we include Construct
     % at the end of the conjunction to give it its value.
@@ -501,10 +499,10 @@
     saved_vars_delay_goal(Conj1, Conj, Construct, Var, no, !SlotInfo),
     conj_list_to_goal(Conj, GoalInfo1, Goal).
 
-    % Push a renamed version of the given construction into the
-    % given goal.  If the goal does not refer to the variable bound
-    % by the construction, then this would have no effect, so we
-    % merely traverse the goal looking for other opportunities.
+    % Push a renamed version of the given construction into the given goal.
+    % If the goal does not refer to the variable bound by the construction,
+    % then this would have no effect, so we merely traverse the goal
+    % looking for other opportunities.
     %
 :- pred push_into_goal_rename(hlds_goal::in, hlds_goal::out, hlds_goal::in,
     prog_var::in, slot_info::in, slot_info::out) is det.
Index: simplify.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/simplify.m,v
retrieving revision 1.271
diff -u -b -r1.271 simplify.m
--- simplify.m	13 Sep 2011 06:07:14 -0000	1.271
+++ simplify.m	13 Sep 2011 06:42:00 -0000
@@ -1949,9 +1949,6 @@
                 ( FinalReason = promise_purity(_)
                 ; FinalReason = from_ground_term(_, _)
                 ; FinalReason = barrier(removable)
-                % XXX: I don't want anything inside the scope moved out of
-                % it or vice versa.  Is this the correct way to ensure this?
-                ; FinalReason = loop_control(_, _)
                 ),
                 Goal = Goal1
             ;
@@ -1964,6 +1961,7 @@
                 ; FinalReason = exist_quant(_)
                 ; FinalReason = promise_solutions(_, _)
                 ; FinalReason = barrier(not_removable)
+                ; FinalReason = loop_control(_, _)
                 ),
                 Goal = Goal1,
                 % Replacing calls, constructions or deconstructions outside
@@ -1976,6 +1974,9 @@
                 % processing the goal inside the commit, to ensure that we
                 % don't make any such replacements when processing the rest
                 % of the goal.
+                %
+                % We do the same for several other kinds of scopes from which
+                % we do not want to "export" common unifications.
                 simplify_info_set_common_info(Common, !Info)
             ;
                 FinalReason = trace_goal(MaybeCompiletimeExpr,
cvs diff: Diffing notes
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------