Re: Unreviewed 3.0 patch to fix loop unrolling

[Bernd Schmidt <bernds at redhat dot com>]

On Mon, 16 Jul 2001, Zoltan Hidvegi wrote:

> I've sent some patches to fix some loop unrolling bugs, that are most
> serious on architectures that use -fbranch-count-reg, e.g. PowerPC.
> Could someone please review it, and approve it or suggest some
> alternative fix?  The patch is
> http://gcc.gnu.org/ml/gcc-patches/2001-07/msg00391.html, fixes PR
> 3384.

There isn't really enough information there to understand what the
patch is doing.

I've run into a few problems with doloop.c and loop unrolling myself on
the ia64.  Could you try whether the patch below helps you?


Bernd

Index: doloop.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/doloop.c,v
retrieving revision 1.5
diff -u -p -r1.5 doloop.c
--- doloop.c	2001/04/11 18:36:50	1.5
+++ doloop.c	2001/07/16 18:03:44
@@ -591,34 +591,37 @@ doloop_modify_runtime (loop, iterations_
 		       copy_rtx (neg_inc ? final_value : initial_value),
 		       NULL_RTX, unsigned_p, OPTAB_LIB_WIDEN);

-  if (loop_info->unroll_number == 1)
+  if (abs_inc * loop_info->unroll_number != 1)
     {
+      int shift_count;
+      rtx extra;
+      rtx label;
+      unsigned HOST_WIDE_INT limit;
+
+      shift_count = exact_log2 (abs_inc * loop_info->unroll_number);
+      if (shift_count < 0)
+	abort ();
+
+      /* abs (final - initial) / abs_inc  */
+      iterations = expand_binop (GET_MODE (diff), lshr_optab,
+				 diff, GEN_INT (shift_count),
+				 NULL_RTX, 1,
+				 OPTAB_LIB_WIDEN);
+
       if (abs_inc != 1)
 	{
-	  int shift_count;
-	  rtx extra;
-	  rtx label;
-
-	  shift_count = exact_log2 (abs_inc);
-	  if (shift_count < 0)
-	    abort ();
-
-	  /* abs (final - initial) / abs_inc  */
-	  iterations = expand_binop (GET_MODE (diff), lshr_optab,
-				     diff, GEN_INT (shift_count),
-				     NULL_RTX, 1,
-				     OPTAB_LIB_WIDEN);
-
 	  /* abs (final - initial) % abs_inc  */
 	  extra = expand_binop (GET_MODE (iterations), and_optab,
-				diff, GEN_INT (abs_inc - 1),
+				diff, GEN_INT (abs_inc * loop_info->unroll_number - 1),
 				NULL_RTX, 1,
 				OPTAB_LIB_WIDEN);

-	  /* If (abs (final - initial) % abs_inc == 0) jump past
-	     following increment instruction.  */
+	  /* If (abs (final - initial) % abs_inc <= abs_inc * (unroll - 1)),
+	     jump past following increment instruction.  */
 	  label = gen_label_rtx();
-	  emit_cmp_and_jump_insns (extra, const0_rtx, EQ, NULL_RTX,
+	  limit = abs_inc * (loop_info->unroll_number - 1);
+	  emit_cmp_and_jump_insns (extra, GEN_INT (limit),
+				   limit == 0 ? EQ : LEU, NULL_RTX,
 				   GET_MODE (extra), 0, 0, label);
 	  JUMP_LABEL (get_last_insn ()) = label;
 	  LABEL_NUSES (label)++;
@@ -631,27 +634,9 @@ doloop_modify_runtime (loop, iterations_

 	  emit_label (label);
 	}
-      else
-	iterations = diff;
     }
   else
-    {
-      int shift_count;
-
-      /* precondition_loop_p has preconditioned the loop so that the
-	 iteration count of the loop body is always a power of 2.
-	 Since we won't get an overflow calculating the loop count,
-	 the code we emit is simpler.  */
-      shift_count = exact_log2 (loop_info->unroll_number * abs_inc);
-      if (shift_count < 0)
-	abort ();
-
-      iterations = expand_binop (GET_MODE (diff), lshr_optab,
-				 diff, GEN_INT (shift_count),
-				 NULL_RTX, 1,
-				 OPTAB_LIB_WIDEN);
-    }
-
+    iterations = diff;

   /* If there is a NOTE_INSN_LOOP_VTOP, we have a `for' or `while'
      style loop, with a loop exit test at the start.  Thus, we can