[m-rev.] for review: --trace shallow variable liveness problem (2)

[Peter Wang]

Branches: main

Revert a change committed on 2009-08-21 to prevent a compiler abort when using
--trace shallow on a test case.  The change caused aborts with other modules.
Fix the original bug a different way, and another bug with --trace shallow.

compiler/live_vars.m:
        Revert the old change.

        Add a check that variables needed across a call do not appear in the
        post-death set of that call.

        Only add typeinfo variables to the set of variables needed across a
        call for output variable which are not local to the call.  Otherwise
        the sanity check fails on livevars_shallow2.m.

compiler/liveness.m:
        A switch variable which needs to be added to the pre-death set of a
        case may require typeinfo variables which describe the type of that
        variable to be added to the pre-death set as well.  This fixes the
        problem with livevars_shallow.m.

tests/valid/livevars_shallow.m:
        Replace this test case by another derived from the same original file.

tests/valid/Mercury.options:
tests/valid/Mmakefile:
tests/valid/livevars_shallow2.m:
        Add test case.

diff --git a/compiler/live_vars.m b/compiler/live_vars.m
index 6e5edeb..fb8e2cb 100644
--- a/compiler/live_vars.m
+++ b/compiler/live_vars.m
@@ -475,30 +475,36 @@ build_live_sets_in_call(OutVars, GoalInfo0, GoalInfo, ResumeVars0, AllocData,
     set.difference(Liveness, OutVars, ForwardVars0),
 
     % Might need to add more live variables with typeinfo liveness
-    % calculation.
+    % calculation. We only add the typeinfo variables for nonlocal outputs.
 
+    NonLocals = goal_info_get_nonlocals(GoalInfo0),
+    set.intersect(OutVars, NonLocals, NonLocalOutVars),
     maybe_add_typeinfo_liveness(AllocData ^ ad_proc_info,
-        AllocData ^ ad_typeinfo_liveness, OutVars, ForwardVars0, ForwardVars),
+        AllocData ^ ad_typeinfo_liveness, NonLocalOutVars,
+        ForwardVars0, ForwardVars),
+
+    % Sanity check.
+    goal_info_get_post_deaths(GoalInfo0, PostDeaths0),
+    set.intersect(PostDeaths0, ForwardVars, Intersect),
+    ( set.empty(Intersect) ->
+        true
+    ;
+        unexpected(this_file,
+            "build_live_sets_in_call: ForwardVars intersects post-death set")
+    ),
 
     Detism = goal_info_get_determinism(GoalInfo0),
     (
         Detism = detism_erroneous,
         AllocData ^ ad_opt_no_return_calls = yes
     ->
-        NeedAcrossCall = need_across_call(set.init, set.init, set.init),
-        GoalInfo1 = GoalInfo0
+        NeedAcrossCall = need_across_call(set.init, set.init, set.init)
     ;
         NeedAcrossCall = need_across_call(ForwardVars, ResumeVars0,
-            !.NondetLiveness),
-        % Make sure a variable doesn't appear in both the forward vars and
-        % post-death set.  (Perhaps the same thing needs to be done for
-        % other need_across_call sets?)
-        goal_info_get_post_deaths(GoalInfo0, PostDeaths0),
-        set.difference(PostDeaths0, ForwardVars, PostDeaths1),
-        goal_info_set_post_deaths(PostDeaths1, GoalInfo0, GoalInfo1)
+            !.NondetLiveness)
     ),
 
-    record_call_site(NeedAcrossCall, GoalInfo1, GoalInfo, !StackAlloc),
+    record_call_site(NeedAcrossCall, GoalInfo0, GoalInfo, !StackAlloc),
 
     % If this is a nondet call, then all the stack slots we need
     % must be protected against reuse in following code.
diff --git a/compiler/liveness.m b/compiler/liveness.m
index a62f656..37352c1 100644
--- a/compiler/liveness.m
+++ b/compiler/liveness.m
@@ -865,10 +865,12 @@ detect_deadness_in_disj([Goal0 | Goals0], [Goal | Goals], Deadness0, Liveness0,
     set(prog_var)::in, set(prog_var)::out, set(prog_var)::out) is det.
 
 detect_deadness_in_cases(SwitchVar, [], [], _Deadness0, _Liveness,
-        CompletedNonLocals, _LiveInfo, !Union, CompletedNonLocalUnion) :-
-    % If the switch variable does not become dead in a case,
-    % it must be put in the pre-death set of that case.
-    set.insert(!.Union, SwitchVar, !:Union),
+        CompletedNonLocals, LiveInfo, !Union, CompletedNonLocalUnion) :-
+    % If the switch variable does not become dead in a case, it must be put in
+    % the pre-death set of that case.
+    maybe_complete_with_typeinfos(LiveInfo, set.make_singleton_set(SwitchVar),
+        CompletedSwitchVar),
+    set.union(CompletedSwitchVar, !Union),
     set.intersect(!.Union, CompletedNonLocals, CompletedNonLocalUnion).
 detect_deadness_in_cases(SwitchVar, [Case0 | Cases0], [Case | Cases],
         Deadness0, Liveness0, CompletedNonLocals, LiveInfo, !Union,
diff --git a/tests/valid/Mercury.options b/tests/valid/Mercury.options
index ab37092..7e87400 100644
--- a/tests/valid/Mercury.options
+++ b/tests/valid/Mercury.options
@@ -97,6 +97,7 @@ MCFLAGS-intermod_user_sharing_2	= --intermodule-optimization
 MCFLAGS-lambda_inference	= --infer-all
 MCFLAGS-livevals_seq		= -O5 --opt-space
 MCFLAGS-livevars_shallow	= --grade none --trace shallow
+MCFLAGS-livevars_shallow2	= --grade none --trace shallow
 MCFLAGS-lco_term		= --optimize-constructor-last-call
 MCFLAGS-loop_inv_bug		= --common-struct --loop-invariants
 MCFLAGS-mc_bag			= --prop-mode-constraints
diff --git a/tests/valid/Mmakefile b/tests/valid/Mmakefile
index e0107ee..111d771 100644
--- a/tests/valid/Mmakefile
+++ b/tests/valid/Mmakefile
@@ -264,6 +264,7 @@ LLDS_PROGS= \
 	exists_cast_bug \
 	fzn_debug_abort \
 	livevars_shallow \
+	livevars_shallow2 \
 	untuple_bug
 
 # These tests only work in grades that support tabling.
diff --git a/tests/valid/livevars_shallow.m b/tests/valid/livevars_shallow.m
index e7763d4..c67d73f 100644
--- a/tests/valid/livevars_shallow.m
+++ b/tests/valid/livevars_shallow.m
@@ -10,45 +10,50 @@
 :- module livevars_shallow.
 :- interface.
 
-:- pred update({}::out) is det.
+:- import_module list.
 
-%-----------------------------------------------------------------------------%
-%-----------------------------------------------------------------------------%
-
-:- implementation.
-
-:- type net(AtomicTask)
-    --->    net.
+:- type node(A)
+    --->    place
+    ;       task
+    .
 
-:- type task(AtomicTask)
-    --->    task.
-
-:- type task_type(AtomicTask)
-    --->    atomik
+:- type task_type(A)
+    --->    atomick
     ;       composite.
 
+:- pred cancellation_set_updates(list(node(A))::in, list(string)::out) is det.
+
+%-----------------------------------------------------------------------------%
 %-----------------------------------------------------------------------------%
 
-update(AtomicTasks) :-
-    Net = net : net(int),
-    classify_nodes(Net, AtomicTasks).
+:- implementation.
+
+cancellation_set_updates(Nodes, L) :-
+    classify_nodes(Nodes, L).
 
-:- pred classify_nodes(net(AtomicTask)::in, {}::out) is det.
+:- pred classify_nodes(list(node(A))::in, list(string)::out) is det.
 
-classify_nodes(_ : net(AtomicTask), AtomicTasks) :-
-    Task = task : task(AtomicTask),
-    TaskType = get_task_type(Task),
+classify_nodes([], []).
+classify_nodes([Node | Nodes], L) :-
     (
-        TaskType = atomik,
-        AtomicTasks = {}
+        Node = place,
+        classify_nodes(Nodes, L)
     ;
-        TaskType = composite,
-        AtomicTasks = {}
+        Node = task,
+        classify_nodes(Nodes, L0),
+        TaskType = get_task_type(Node),
+        (
+            TaskType = atomick,
+            L = L0
+        ;
+            TaskType = composite,
+            L = L0
+        )
     ).
 
-:- func get_task_type(task(AtomicTask)) = task_type(AtomicTask).
+:- func get_task_type(node(A)) = task_type(A).
 
-get_task_type(_) = atomik.
+get_task_type(_) = composite.
 
 %-----------------------------------------------------------------------------%
 % vim: set sts=4 sw=4 et:
diff --git a/tests/valid/livevars_shallow2.m b/tests/valid/livevars_shallow2.m
new file mode 100644
index 0000000..00793f4
--- /dev/null
+++ b/tests/valid/livevars_shallow2.m
@@ -0,0 +1,50 @@
+%-----------------------------------------------------------------------------%
+% Regression test.
+% The compiler aborted on this module with --trace shallow due to an incorrect
+% fix.
+%-----------------------------------------------------------------------------%
+
+:- module livevars_shallow2.
+:- interface.
+
+:- import_module io.
+
+:- pred main(io::di, io::uo) is det.
+
+%-----------------------------------------------------------------------------%
+%-----------------------------------------------------------------------------%
+
+:- implementation.
+
+:- type ps ---> ps(int).
+
+%-----------------------------------------------------------------------------%
+
+main(!IO) :-
+    P = stringify_state(int_with_state),
+    io.write(P, !IO),
+    io.nl(!IO).
+
+:- pred int_with_state(int::out, ps::in, ps::out) is semidet.
+
+int_with_state(1, !PS) :-
+    semidet_true.
+
+:- pred stringify_state(
+        pred(T, ps, ps)::
+            in(pred(out, in, out) is semidet),
+        string::out,
+        ps::in,
+        ps::out)
+        is semidet.
+
+stringify_state(P, String, !PS) :-
+    P(_, !PS),
+    String = xfoo(-1).
+
+:- func xfoo(T) = string.
+
+xfoo(_) = "xfoo".
+
+%-----------------------------------------------------------------------------%
+% vim: set sts=4 sw=4 et:


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