comments on loop changes

Richard Henderson rth@cygnus.com
Sat Feb 13 17:37:00 GMT 1999


On Sat, Feb 13, 1999 at 01:33:55PM +1300, Michael Hayes wrote:
> Now gcc-2.8.1 still generates much better code for the c4x since it
> has the test in combine_givs_p to reject GIVs for combination based on
> the address cost.  With your understandable reluctance to revert your
> patch to combine_givs_p, the alternative is to modify
> combine_givs_benefit_from.  Richard, could you supply a comment for
> this function to specify how it should behave?

It's goal was to account for and remove unintentional benefit from a
giv that is used only once from the single giv in which it is used.

However, I now believe it to be a mistake to allow any giv that is
used only once to be combined with anything.  Any subsequent giv that
it might combine with gains nothing that could not have been gotten
cheaper by simply reducing the second giv.

This losage appears on any target supporting reg+reg.  Consider

    short bar[100][100];
    void foo()
    {
      int i, j;
    
      for (i = 0; i < 100; ++i)
        for (j = 0; j < 100; ++j)
          bar[i][j] = 0;
    }

on x86, we get in the inner loop

    Insn 31: giv reg 26 src reg 23 benefit 7 lifetime 2 replaceable ncav
	     mult 2 add (reg:SI 25)
    Insn 35: dest address src reg 23 benefit 8 lifetime 1 replaceable ncav
 	     mult 2 add (plus:SI (reg:SI 25) (reg:SI 27))
    giv at 35 combined with giv at 31
    giv at 35 reduced to (plus:SI (reg:SI 31) (reg:SI 27))

and

  .L10:
        leal (%ebx,%edx),%eax
        movw $0,(%edi,%eax)
        addl $-2,%edx
        decl %ecx
        jns .L10

Blech.  By denying such a combination we get

  .L10:
        movw $0,(%eax)
        addl $-2,%eax
        decl %ecx
        jns .L10


I need to take the time to reexamine your c4x test cases...

Comments?


r~


	* loop.c (combine_givs_used_by_other): Delete.
	(combine_givs_benefit_from): Delete.
	(combine_givs): Deny combination of givs only used once.  Simplify
	code with the death of combine_givs_benefit_from.

Index: loop.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/loop.c,v
retrieving revision 1.135
diff -c -p -d -r1.135 loop.c
*** loop.c	1999/02/13 14:25:58	1.135
--- loop.c	1999/02/14 01:19:21
*************** cmp_combine_givs_stats (x, y)
*** 6768,6809 ****
    return d;
  }
  
- /* If one of these givs is a DEST_REG that was used once by the other giv,
-    this is actually a single use.  Return 0 if this is not the case,
-    -1 if g1 is the DEST_REG involved, and 1 if it was g2.  */
- 
- static int
- combine_givs_used_by_other (g1, g2)
-      struct induction *g1, *g2;
- {
-   if (g1->giv_type == DEST_REG
-       && VARRAY_RTX (reg_single_usage, REGNO (g1->dest_reg)) != 0
-       && VARRAY_RTX (reg_single_usage, REGNO (g1->dest_reg)) != const0_rtx
-       && reg_mentioned_p (g1->dest_reg, PATTERN (g2->insn)))
-     return -1;
- 
-   if (g2->giv_type == DEST_REG
-       && VARRAY_RTX (reg_single_usage, REGNO (g2->dest_reg)) != 0
-       && VARRAY_RTX (reg_single_usage, REGNO (g2->dest_reg)) != const0_rtx
-       && reg_mentioned_p (g2->dest_reg, PATTERN (g1->insn)))
-     return 1;
- 
-   return 0;
- }
-  
- static int
- combine_givs_benefit_from (g1, g2)
-      struct induction *g1, *g2;
- {
-   int tmp = combine_givs_used_by_other (g1, g2);
-   if (tmp < 0)
-     return 0;
-   else if (tmp > 0)
-     return g2->benefit - g1->benefit;
-   else
-     return g2->benefit;
- }
- 
  /* Check all pairs of givs for iv_class BL and see if any can be combined with
     any other.  If so, point SAME to the giv combined with and set NEW_REG to
     be an expression (in terms of the other giv's DEST_REG) equivalent to the
--- 6757,6762 ----
*************** static void
*** 6813,6818 ****
--- 6766,6774 ----
  combine_givs (bl)
       struct iv_class *bl;
  {
+   /* Additional benefit to add for being combined multiple times.  */
+   const int extra_benefit = 3;
+ 
    struct induction *g1, *g2, **giv_array;
    int i, j, k, giv_count;
    struct combine_givs_stats *stats;
*************** combine_givs (bl)
*** 6840,6852 ****
--- 6796,6822 ----
    for (i = 0; i < giv_count; i++)
      {
        int this_benefit;
+       rtx single_use;
  
        g1 = giv_array[i];
+       stats[i].giv_number = i;
+ 
+       /* If a DEST_REG GIV is used only once, do not allow it to combine
+ 	 with anything, for in doing so we will gain nothing that cannot
+ 	 be had by simply letting the GIV with which we would have combined
+ 	 to be reduced on its own.  The losage shows up in particular with 
+ 	 DEST_ADDR targets on hosts with reg+reg addressing, though it can
+ 	 be seen elsewhere as well.  */
+       if (g1->giv_type == DEST_REG
+ 	  && (single_use = VARRAY_RTX (reg_single_usage, REGNO (g1->dest_reg)))
+ 	  && single_use != const0_rtx)
+ 	continue;
  
        this_benefit = g1->benefit;
        /* Add an additional weight for zero addends.  */
        if (g1->no_const_addval)
  	this_benefit += 1;
+ 
        for (j = 0; j < giv_count; j++)
  	{
  	  rtx this_combine;
*************** combine_givs (bl)
*** 6856,6867 ****
  	      && (this_combine = combine_givs_p (g1, g2)) != NULL_RTX)
  	    {
  	      can_combine[i*giv_count + j] = this_combine;
! 	      this_benefit += combine_givs_benefit_from (g1, g2);
! 	      /* Add an additional weight for being reused more times.  */
! 	      this_benefit += 3;
  	    }
  	}
-       stats[i].giv_number = i;
        stats[i].total_benefit = this_benefit;
      }
  
--- 6826,6834 ----
  	      && (this_combine = combine_givs_p (g1, g2)) != NULL_RTX)
  	    {
  	      can_combine[i*giv_count + j] = this_combine;
! 	      this_benefit += g2->benefit + extra_benefit;
  	    }
  	}
        stats[i].total_benefit = this_benefit;
      }
  
*************** restart:
*** 6908,6914 ****
  	      g1->combined_with++;
  	      g1->lifetime += g2->lifetime;
  
! 	      g1_add_benefit += combine_givs_benefit_from (g1, g2);
  
  	      /* ??? The new final_[bg]iv_value code does a much better job
  		 of finding replaceable giv's, and hence this code may no
--- 6875,6881 ----
  	      g1->combined_with++;
  	      g1->lifetime += g2->lifetime;
  
! 	      g1_add_benefit += g2->benefit;
  
  	      /* ??? The new final_[bg]iv_value code does a much better job
  		 of finding replaceable giv's, and hence this code may no
*************** restart:
*** 6922,6932 ****
  		{
  		  int m = stats[l].giv_number;
  		  if (can_combine[m*giv_count + j])
! 		    {
! 		      /* Remove additional weight for being reused.  */
! 		      stats[l].total_benefit -= 3 + 
! 			combine_givs_benefit_from (giv_array[m], g2);
! 		    }
  		}
  
  	      if (loop_dump_stream)
--- 6889,6895 ----
  		{
  		  int m = stats[l].giv_number;
  		  if (can_combine[m*giv_count + j])
! 		    stats[l].total_benefit -= g2->benefit + extra_benefit;
  		}
  
  	      if (loop_dump_stream)
*************** restart:
*** 6944,6954 ****
  	    {
  	      int m = stats[j].giv_number;
  	      if (can_combine[m*giv_count + j])
! 		{
! 		  /* Remove additional weight for being reused.  */
! 		  stats[j].total_benefit -= 3 + 
! 		    combine_givs_benefit_from (giv_array[m], g1);
! 		}
  	    }
  
  	  g1->benefit += g1_add_benefit;
--- 6907,6913 ----
  	    {
  	      int m = stats[j].giv_number;
  	      if (can_combine[m*giv_count + j])
! 		stats[j].total_benefit -= g1->benefit + extra_benefit;
  	    }
  
  	  g1->benefit += g1_add_benefit;


More information about the Gcc-patches mailing list