for review: loop invariant hoisting bug

[Peter Wang-6]

Branches: main

compiler/loop_inv.m:
        Fix a bug.  An `unused' variable in an invariant call was treated
        like an output variable and added to the invariant variables list,
        and hoisted.

tests/hard_coded/Mercury.options:
tests/hard_coded/loop_inv_test3.exp:
tests/hard_coded/loop_inv_test3.m:
        Add test case.

diff --git a/compiler/loop_inv.m b/compiler/loop_inv.m
index 9b13fad..7961854 100644
--- a/compiler/loop_inv.m
+++ b/compiler/loop_inv.m
@@ -232,7 +232,7 @@ hoist_loop_invariants(PredId, ProcId, PredInfo, !ProcInfo, !ModuleInfo) :-
             AuxPredInfo, AuxProcInfo, !ModuleInfo),
 
         % We update the body of AuxProc by replacing adding the set of
-        % computed invariant vars to the argument list, % replacing invariant
+        % computed invariant vars to the argument list, replacing invariant
         % goals in InProc with `true', and recursive calls at the end of
         % recursive paths with calls to the auxiliary procedure.
         gen_aux_proc(InvGoals, PredProcId,
@@ -1211,8 +1211,8 @@ input_arg(ModuleInfo, X, M) = X :-
 
 %-----------------------------------------------------------------------------%
 
-    % Find the list of vars for a goal that are free before the call.
-    % This only applies to calls and unifications.
+    % Find the list of vars for a goal that are free before the call and bound
+    % afterwards.  This only applies to calls and unifications.
     %
 :- func goal_outputs(module_info, hlds_goal) = prog_vars.
 
@@ -1269,13 +1269,12 @@ goal_outputs(ModuleInfo, Goal) = Outputs :-
 
 %-----------------------------------------------------------------------------%
 
-    % An output arg is one whose pre-call inst is free.
+    % An output arg is one whose pre-call inst is free and ground after.
     %
 :- func output_arg(module_info, prog_var, mer_mode) = prog_var is semidet.
 
 output_arg(ModuleInfo, X, M) = X :-
-    mode_get_insts(ModuleInfo, M, InInst, _OutInst),
-    inst_is_free(ModuleInfo, InInst).
+    mode_is_fully_output(ModuleInfo, M).
 
 %-----------------------------------------------------------------------------%
 
diff --git a/tests/hard_coded/Mercury.options b/tests/hard_coded/Mercury.options
index f3160e9..fe3d47b 100644
--- a/tests/hard_coded/Mercury.options
+++ b/tests/hard_coded/Mercury.options
@@ -60,6 +60,7 @@ MCFLAGS-loop_inv_test = --loop-invariants --trace-optimized
 MCFLAGS-loop_inv_test0 = --loop-invariants --trace-optimized
 MCFLAGS-loop_inv_test1 = --loop-invariants --trace-optimized
 MCFLAGS-loop_inv_test2 = --loop-invariants --trace-optimized
+MCFLAGS-loop_inv_test3 = --loop-invariants --trace-optimized
 MCFLAGS-prince_frameopt = --intermodule-optimization -O5
 MCFLAGS-prince_frameopt_css = --intermodule-optimization -O5
 MCFLAGS-prince_frameopt_css.style = --intermodule-optimization -O5
diff --git a/tests/hard_coded/loop_inv_test3.exp b/tests/hard_coded/loop_inv_test3.exp
new file mode 100644
index 0000000..6a2839f
--- /dev/null
+++ b/tests/hard_coded/loop_inv_test3.exp
@@ -0,0 +1 @@
+[100, 100, 100, 100, 100, 100, 100, 100, 100, 100]
diff --git a/tests/hard_coded/loop_inv_test3.m b/tests/hard_coded/loop_inv_test3.m
new file mode 100644
index 0000000..4b00f99
--- /dev/null
+++ b/tests/hard_coded/loop_inv_test3.m
@@ -0,0 +1,50 @@
+%-----------------------------------------------------------------------------%
+% Regression test.  Loop invariant hoisting incorrectly treated `unused'
+% arguments in calls like outputs.
+
+:- module loop_inv_test3.
+:- interface.
+
+:- import_module io.
+
+:- pred main(io::di, io::uo) is det.
+
+%-----------------------------------------------------------------------------%
+%-----------------------------------------------------------------------------%
+
+:- implementation.
+
+:- import_module int.
+:- import_module list.
+
+%-----------------------------------------------------------------------------%
+
+main(!IO) :-
+    loop(10, 10, [], L),
+    io.write(L, !IO),
+    io.nl(!IO).
+
+:- pred loop(int::in, int::in, list(int)::in, list(int)::out) is det.
+
+loop(Factor, N, L0, L) :-
+    Value = foo(Factor, N1),
+    bar(N, N1),
+    ( N1 = 0 ->
+        L = L0
+    ;
+        L1 = [Factor * Value | L0],
+        loop(Factor, N - 1, L1, L)
+    ).
+
+:- func foo(int::in, int::unused) = (int::out) is det.
+:- pragma no_inline(foo/2).
+
+foo(F, _) = F.
+
+:- pred bar(int::in, int::out) is det.
+:- pragma no_inline(bar/2).
+
+bar(N, N).
+
+%-----------------------------------------------------------------------------%
+% vim: ft=mercury ts=8 sts=4 sw=4 et

--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews@...
Administrative Queries: owner-mercury-reviews@...
Subscriptions:          mercury-reviews-request@...
--------------------------------------------------------------------------

[Zoltan Somogyi-2]

On 23-Oct-2009, Peter Wang <novalazy@...> wrote:
> compiler/loop_inv.m:
>         Fix a bug.  An `unused' variable in an invariant call was treated
>         like an output variable and added to the invariant variables list,
>         and hoisted.

That looks good.

Zoltan.
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews@...
Administrative Queries: owner-mercury-reviews@...
Subscriptions:          mercury-reviews-request@...
--------------------------------------------------------------------------