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]

PATCH: Fix PR2162



Here's another draft patch for the SPARC.  Phil, if you're willing to
run another bootstrap/test cycle when the one you're doing gets done,
that would be excellent.  Richard, borrowing a few more of your brain
cycles would be cool.

The problem is that loop-8.c failed with -O3 -fPIC on the SPARC.  The
bottom line is that loop decided to reverse this loop:

  for (d = 0; d < 3; d++)
  {
    c = a[d];
    if (c > 0.0) goto e;
  }

Uncool.

The problem is this conditional:

      if ((num_nonfixed_reads <= 1
	   && ! loop_info->has_nonconst_call
	   && ! loop_info->has_volatile
	   && reversible_mem_store
	   && (bl->giv_count + bl->biv_count + loop_info->num_mem_sets
	       + LOOP_MOVABLES (loop)->num + compare_and_branch == insn_count)
	   && (bl == ivs->list && bl->next == 0))
	  || no_use_except_counting)

in check_dbra_loop.  The important part is the bit that tries to
determine that all the instructions in the loop are accounted for:
they are either set bivs or givs based on the loop counter, set
memory, are a movable instruction, etc.

The problem is that LOOP_MOVABLES (loop)->num at this point refers to
all the movables in existence -- including some that have been moved
out of the loop -- but insn_count only corresponds to instructions
within the loop.  So, we think all the instructions are accounted for,
but that's not really correct.

The logic setting ->num seemed fragile, so I decided just to compute
the right number when we need it.  I verified that this fixed the
loop-8 bug by hand-inspecting the assembly code, but I'm not sure that
I'm a compliant SPARC processor.  I also checked that we still reverse
some simple loops.

If I got things wrong, we could end up reversing other loops that
we're not supposed to, which would be bad.  (The other failure mode --
not reversing things we should -- is not as bad.)  So, even though I'm
bootstrapping on i86-pc-linux-gnu, I'd appreciate a SPARC bootstrap as
well.

Thanks,

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Index: loop.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/loop.c,v
retrieving revision 1.322.4.7
diff -c -p -r1.322.4.7 loop.c
*** loop.c	2001/04/25 15:29:38	1.322.4.7
--- loop.c	2001/05/03 02:06:13
*************** static void ignore_some_movables PARAMS 
*** 166,171 ****
--- 166,172 ----
  static void force_movables PARAMS ((struct loop_movables *));
  static void combine_movables PARAMS ((struct loop_movables *,
  				      struct loop_regs *));
+ static int num_unmoved_movables PARAMS ((const struct loop *));
  static int regs_match_p PARAMS ((rtx, rtx, struct loop_movables *));
  static int rtx_equal_for_loop_p PARAMS ((rtx, rtx, struct loop_movables *,
  					 struct loop_regs *));
*************** scan_loop (loop, flags)
*** 551,557 ****
  
    movables->head = 0;
    movables->last = 0;
-   movables->num = 0;
  
    /* Determine whether this loop starts with a jump down to a test at
       the end.  This will occur for a small number of loops with a test
--- 552,557 ----
*************** combine_movables (movables, regs)
*** 1422,1427 ****
--- 1422,1445 ----
    /* Clean up.  */
    free (matched_regs);
  }
+ 
+ /* Returns the number of movable instructions in LOOP that were not
+    moved outside the loop.  */
+ 
+ static int
+ num_unmoved_movables (loop)
+      const struct loop *loop;
+ {
+   int num = 0;
+   struct movable *m;
+ 
+   for (m = LOOP_MOVABLES (loop)->head; m; m = m->next)
+     if (!m->done)
+       ++num;
+ 
+   return num;
+ }
+ 
  
  /* Return 1 if regs X and Y will become the same if moved.  */
  
*************** move_movables (loop, movables, threshold
*** 1635,1642 ****
    rtx *reg_map = (rtx *) xcalloc (nregs, sizeof (rtx));
    char *already_moved = (char *) xcalloc (nregs, sizeof (char));
  
-   movables->num = 0;
- 
    for (m = movables->head; m; m = m->next)
      {
        /* Describe this movable insn.  */
--- 1653,1658 ----
*************** move_movables (loop, movables, threshold
*** 1665,1673 ****
  		     INSN_UID (m->forces->insn));
  	}
  
-       /* Count movables.  Value used in heuristics in strength_reduce.  */
-       movables->num++;
- 
        /* Ignore the insn if it's already done (it matched something else).
  	 Otherwise, see if it is now safe to move.  */
  
--- 1681,1686 ----
*************** check_dbra_loop (loop, insn_count)
*** 7363,7369 ****
  	   && ! loop_info->has_volatile
  	   && reversible_mem_store
  	   && (bl->giv_count + bl->biv_count + loop_info->num_mem_sets
! 	       + LOOP_MOVABLES (loop)->num + compare_and_branch == insn_count)
  	   && (bl == ivs->list && bl->next == 0))
  	  || no_use_except_counting)
  	{
--- 7376,7382 ----
  	   && ! loop_info->has_volatile
  	   && reversible_mem_store
  	   && (bl->giv_count + bl->biv_count + loop_info->num_mem_sets
! 	       + num_unmoved_movables (loop) + compare_and_branch == insn_count)
  	   && (bl == ivs->list && bl->next == 0))
  	  || no_use_except_counting)
  	{
Index: loop.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/loop.h,v
retrieving revision 1.51
diff -c -p -r1.51 loop.h
*** loop.h	2001/01/25 09:28:55	1.51
--- loop.h	2001/05/03 02:06:14
*************** struct loop_movables
*** 289,296 ****
    struct movable *head;
    /* Last movable in chain.  */
    struct movable *last;
-   /* Number of movables in the loop.  */
-   int num;
  };
  
  
--- 289,294 ----


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