[m-rev.] diff: fix a bug in loop invariant hoisting

[Julien Fischer]

On Mon, 20 Sep 2004, Zoltan Somogyi wrote:

> On 20-Sep-2004, Julien Fischer <juliensf at cs.mu.OZ.AU> wrote:
> > The bug was caused by the modes of the extra arguments in
> > foreign procs not being considered.  The current fix
> > is to just make any extra arguments non-hoistable.
>
> That looks fine, but it begs the question of what loop invariant optimization
> was doing with such goals in the first place. IIRC, all foreign_procs with
> extra arguments are marked as impure or semipure, and so shouldn't be optimized
> away by loop invariant optimization.
>

Ralph has already explained why this is happening.  I've modified
the diff to include a longer explanation of what's happening here.

XXX Fixing this properly doesn't look too difficult if anyone
is looking for something to do.

BTW, Zoltan could you please have a look at the XXX comments in
hlds_goal.m at lines 169 and 245.  You modified them during one
of your changes to tabling and they're both incomplete now.

(modified version of diff follows, log message is unchanged)

Julien.

Index: loop_inv.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/loop_inv.m,v
retrieving revision 1.12
diff -u -r1.12 loop_inv.m
--- loop_inv.m	16 Jun 2004 03:44:45 -0000	1.12
+++ loop_inv.m	20 Sep 2004 05:10:04 -0000
@@ -1092,10 +1092,20 @@
 uniquely_used_vars_2(MI, generic_call(_, Args, Modes, _) - _) =
     list__filter_map_corresponding(uniquely_used_args(MI), Args, Modes).

-uniquely_used_vars_2(MI, foreign_proc(_, PredId, ProcId, Args, Extras, _) - _) =
+uniquely_used_vars_2(MI, foreign_proc(_, PredId, ProcId, Args, Extras, _) - _)
+        = Result :-
+    %
+    % XXX `Extras' should be empty for pure calls.  We cannot apply
+    % LIO to non-pure goals so we shoudn't need to consider `Extras'.
+    % However, we currently don't deal with the situation where we
+    % may be trying to apply LIO to a non-pure goal until *after* we
+    % have called this predicate, so `Extras' may not be empty.  As a
+    % work-around we just add any variables in `Extras' to the set of
+    % variables that cannot be hoisted.
+    %
     list__filter_map_corresponding(uniquely_used_args(MI),
-        list__map(foreign_arg_var, Args ++ Extras),
-        argmodes(MI,PredId,ProcId)).
+        list__map(foreign_arg_var, Args),
+        argmodes(MI,PredId,ProcId)) ++ list.map(foreign_arg_var, Extras).

     % XXX This is very conservative!
     %
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------