This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RFC: PATCH to unroll_loop for optimization/10171


In general, unroll_loop doesn't try to unroll loops that were inverted by
expand_end_loop (which seems odd, since that should happen for most loops).
But it does try to strip the jumps from a loop which is known to have only
a single iteration.

To get this right, we need to be able to determine whether or not an
unconditional jump at the beginning of the loop is going to the loop
condition.  But we don't record where the loop condition is, and we can't
deduce it from the RTL.

The current code tests for a jump to the continue point, which works for a
while loop, but is wrong for a for loop, where the continue point is the
beginning of the increment expression.  So it erroneously concludes that
the jump is not to the condition and leaves it alone, but it deletes the
conditional jump, so we execute the loop zero times rather than the once we
were supposed to.  Oops.

It seems to me that fixing this in the current model will require adding
yet another loop note so that we can identify when we're jumping to the
condition.  A better model wouldn't care; we should be using flow/biv
information here, not instruction pattern matching.  It shouldn't matter
whether the loop was written

  for (; i < 2; ++i) { ... }

and rotated by the compiler, or explicitly written

  while (1) { goto test; ...; ++i; test: if (i>=2) break; }

A conservative fix is just to disable the optimization.  Should I go ahead
and apply this, or does someone want to work on a more involved fix?
Perhaps a more involved fix should only go into the trunk, and we should
use the conservative fix for 3.[23].

2003-03-21  Jason Merrill  <jason at redhat dot com>

	PR optimization/10171
	* unroll.c (unroll_loop): Disable code for handling a single
	iteration.

*** unroll.c.~1~	2003-03-10 11:45:17.000000000 -0500
--- unroll.c	2003-03-21 01:29:37.000000000 -0500
*************** unroll_loop (loop, insn_count, strength_
*** 299,310 ****
    /* Calculate how many times to unroll the loop.  Indicate whether or
       not the loop is being completely unrolled.  */
  
!   if (loop_info->n_iterations == 1)
      {
        /* Handle the case where the loop begins with an unconditional
  	 jump to the loop condition.  Make sure to delete the jump
  	 insn, otherwise the loop body will never execute.  */
  
        rtx ujump = ujump_to_loop_cont (loop->start, loop->cont);
        if (ujump)
  	delete_related_insns (ujump);
--- 299,315 ----
    /* Calculate how many times to unroll the loop.  Indicate whether or
       not the loop is being completely unrolled.  */
  
!   if (0 && loop_info->n_iterations == 1)
      {
        /* Handle the case where the loop begins with an unconditional
  	 jump to the loop condition.  Make sure to delete the jump
  	 insn, otherwise the loop body will never execute.  */
  
+       /* FIXME THIS IS BROKEN -- it checks for jumps to the continue point
+ 	 rather than to the condition.  In a for loop, these are not the
+ 	 same, so we don't delete the initial jump, so the loop body never
+ 	 executes.  */
+ 
        rtx ujump = ujump_to_loop_cont (loop->start, loop->cont);
        if (ujump)
  	delete_related_insns (ujump);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]