[PATCH] Fix loop hoisting bug on ia32

[Jakub Jelinek]

Hi!

The testcase below fails on ia32 with -O1 and above, loop considers code
after `e' label as dead code and the goto jumps instead before the first bar.
There are actually two issues:
- one is end_label computation: we cannot use next_label(loop->end) as can
be seen in this case, because next_label happily skips over several
instructions and returns the `e' CODE_LABEL.
- the second issue is that we would not emit the hoisting fixup sequence at
that label anyway even if we did not change the goto to a bogus label.
This patch solves this just by not hoisting any MEMs out of loop if there
are gotos out of the loop (not counting jumps right after the end of the
loop). Alternatively, we could still hoist the MEMs if all the outside
labels were preceeded by BARRIERs and we would replicate the fixup sequences
before all those labels. If we are to do that, I think we should have some
upper bound on the number of moves replicated that way (ie. number of
different outside labels times number of hoisted MEMs) so that the resulting
code does not get too big.
Is it ok to commit this and perhaps if you want later optimize as described
above (ideas on what should be the upper bound?)?
BTW: Current CVS is not bootstrapping on i386 for me at the moment (no matter
whether this patch is in or not fails during zlib compilation).

2000-11-28  Jakub Jelinek  <jakub@redhat.com>

	* loop.c (load_mems): Avoid using next_label to find end_label. If
	jumping outside of the loop (other than loop end), don't hoist MEMs
	out of loop.

	* gcc.c-torture/execute/loop-8.c: New test.

--- gcc/loop.c.jj	Tue Nov 28 10:05:42 2000
+++ gcc/loop.c	Tue Nov 28 12:57:06 2000
@@ -8716,7 +8716,7 @@ load_mems (loop)
   int i;
   rtx p;
   rtx label = NULL_RTX;
-  rtx end_label = NULL_RTX;
+  rtx end_label;
   /* Nonzero if the next instruction may never be executed.  */
   int next_maybe_never = 0;
   int last_max_reg = max_reg_num ();
@@ -8724,21 +8724,14 @@ load_mems (loop)
   if (loop_info->mems_idx == 0)
     return;
 
-  /* Find start of the extended basic block that enters the loop.  */
-  for (p = loop->start;
-       PREV_INSN (p) && GET_CODE (p) != CODE_LABEL;
-       p = PREV_INSN (p))
-    ;
-
-  cselib_init ();
-
-  /* Build table of mems that get set to constant values before the
-     loop.  */
-  for (; p != loop->start; p = NEXT_INSN (p))
-    cselib_process_insn (p);
-
-  /* Check to see if it's possible that some instructions in the
-     loop are never executed.  */
+  /* We cannot use next_label here because it skips over normal insns.  */
+  end_label = next_nonnote_insn (loop->end);
+  if (end_label && GET_CODE (end_label) != CODE_LABEL)
+    end_label = NULL_RTX;
+
+  /* Check to see if it's possible that some instructions in the loop are
+     never executed.  Also check if there is a goto out of the loop other
+     than right after the end of the loop.  */
   for (p = next_insn_in_loop (loop, loop->scan_start);
        p != NULL_RTX && ! maybe_never;
        p = next_insn_in_loop (loop, p))
@@ -8757,6 +8750,15 @@ load_mems (loop)
 		     && NEXT_INSN (NEXT_INSN (p)) == loop->end
 		     && any_uncondjump_p (p)))
 	{
+	  /* If this is a jump outside of the loop but not right
+	     after the end of the loop, we would have to emit new fixup
+	     sequences for each such label.  */
+	  if (JUMP_LABEL (p) != end_label
+	      && (INSN_UID (JUMP_LABEL (p)) >= max_uid_for_loop
+		  || INSN_LUID (JUMP_LABEL (p)) < INSN_LUID (loop->start)
+		  || INSN_LUID (JUMP_LABEL (p)) > INSN_LUID (loop->end)))
+	    return;
+
 	  if (!any_condjump_p (p))
 	    /* Something complicated.  */
 	    maybe_never = 1;
@@ -8769,6 +8771,19 @@ load_mems (loop)
 	maybe_never = 1;
     }
 
+  /* Find start of the extended basic block that enters the loop.  */
+  for (p = loop->start;
+       PREV_INSN (p) && GET_CODE (p) != CODE_LABEL;
+       p = PREV_INSN (p))
+    ;
+
+  cselib_init ();
+
+  /* Build table of mems that get set to constant values before the
+     loop.  */
+  for (; p != loop->start; p = NEXT_INSN (p))
+    cselib_process_insn (p);
+
   /* Actually move the MEMs.  */
   for (i = 0; i < loop_info->mems_idx; ++i)
     {
@@ -8960,10 +8975,6 @@ load_mems (loop)
 	    {
 	      if (label == NULL_RTX)
 		{
-		  /* We must compute the former
-		     right-after-the-end label before we insert
-		     the new one.  */
-		  end_label = next_label (loop->end);
 		  label = gen_label_rtx ();
 		  emit_label_after (label, loop->end);
 		}
@@ -9001,7 +9012,7 @@ load_mems (loop)
 	}
     }
 
-  if (label != NULL_RTX)
+  if (label != NULL_RTX && end_label != NULL_RTX)
     {
       /* Now, we need to replace all references to the previous exit
 	 label with the new one.  */
--- gcc/testsuite/gcc.c-torture/execute/loop-8.c.jj	Tue Nov 28 13:02:09 2000
+++ gcc/testsuite/gcc.c-torture/execute/loop-8.c	Mon Nov 27 23:40:49 2000
@@ -0,0 +1,23 @@
+double a[3] = { 0.0, 1.0, 2.0 };
+
+void bar (int x, double *y)
+{
+  if (x || *y != 1.0)
+    abort ();
+}
+
+int main ()
+{
+  double c;
+  int d;
+  for (d = 0; d < 3; d++)
+  {
+    c = a[d];
+    if (c > 0.0) goto e;
+  }
+  bar(1, &c);
+  exit (1);
+e:
+  bar(0, &c);
+  exit (0);
+}

	Jakub