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]

Re: [patch] Miscelaneous ivopts improvements (1 of 3)


Hello,

> > > + /* Strips constant offsets from EXPR and stores them to OFFSET.  If INSIDE_ADDR
> > > +    is true, assume we are inside an address.  If MAY_STRIP_NOPS is true,
> > > +    also strip nops if useful.  */
> > > + 
> > > + static tree
> > > + strip_offset (tree expr, bool inside_addr, bool may_strip_nops,
> > > + 	      unsigned HOST_WIDE_INT *offset)
> > You need to document the return value of this function.
> > 
> > When is it safe to strip nops?  I see that it is considered unsafe to
> > do so for the call from add_address_candicates, but that it is 
> > considered safe to strip nops for the calls from difference_cost.
> > It would be useful to know why (I have a suspicion why, but I'd
> > like to hear the explanation in your words).
> 
> the expressions obtained from strip_offset with may_strip_nops = true
> are not typed correctly (the types of operands do not need to match
> type of result of the operation).  So it cannot be used anywhere where
> the produced expression might get emitted to the code.
> 
> The effect of may_strip_nops = true is that the differences in casts
> inside the expressions compared in difference_cost are eliminated,
> thus we have a better chance of noting that they are in fact identical.
> 
> Thinking about the issue again, it seems safer to add the "normalized" casts
> instead, to keep the types in expressions correct while gaining the
> desired effect in difference_cost.  I will change this, extend the
> comments and resubmit the patch.

here is the updated version (re-bootstrapped & regtested on i686).
The strip_offset function now does not produce the wrong-typed
expressions it did in the last version.

Additionally, a small fix to determine_base_object is included -- this
is to make help PR19701 more.  One of the problems in th PR is this:
ivopts don't allow pointer to an array to be used to address memory in
a different array (which caused problems for rtl alias analysis and in
general usually was not a good idea).  To ensure this, we determine
a base object for the reference.  However for references with addresses
of type (cast) (pointer + 8) and (cast) (pointer + 16) this caused
us to use two different ivs, since we did not handle casts in
determine_base_object and thus considered these references to point to
different objects.

I may send this as a separate patch if you prefer, however since the
issues are somewhat related, I decided to include this part here.

Zdenek

	* tree-ssa-loop-ivopts.c (determine_base_object): Ignore casts.
	(strip_offset): Handle addresses.
	(add_address_candidates): Use strip_offset.
	(difference_cost): Reflect strip_offset change.

Index: tree-ssa-loop-ivopts.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-loop-ivopts.c,v
retrieving revision 2.44
diff -c -3 -p -r2.44 tree-ssa-loop-ivopts.c
*** tree-ssa-loop-ivopts.c	6 Feb 2005 18:48:58 -0000	2.44
--- tree-ssa-loop-ivopts.c	6 Feb 2005 19:26:52 -0000
*************** determine_base_object (tree expr)
*** 707,712 ****
--- 707,716 ----
  
        return fold (build (code, ptr_type_node, op0, op1));
  
+     case NOP_EXPR:
+     case CONVERT_EXPR:
+       return determine_base_object (TREE_OPERAND (expr, 0));
+ 
      default:
        return fold_convert (ptr_type_node, expr);
      }
*************** find_interesting_uses (struct ivopts_dat
*** 1675,1680 ****
--- 1679,1781 ----
    free (body);
  }
  
+ /* Strips constant offsets from EXPR and stores them to OFFSET.  If INSIDE_ADDR
+    is true, assume we are inside an address.  */
+ 
+ static tree
+ strip_offset (tree expr, bool inside_addr, unsigned HOST_WIDE_INT *offset)
+ {
+   tree op0 = NULL_TREE, op1 = NULL_TREE, step;
+   enum tree_code code;
+   tree type, orig_type = TREE_TYPE (expr);
+   unsigned HOST_WIDE_INT off0, off1, st;
+ 
+   STRIP_NOPS (expr);
+   type = TREE_TYPE (expr);
+   code = TREE_CODE (expr);
+   *offset = 0;
+ 
+   switch (code)
+     {
+     case INTEGER_CST:
+       if (!cst_and_fits_in_hwi (expr))
+ 	return fold_convert (orig_type, expr);
+ 
+       *offset = int_cst_value (expr);
+       return build_int_cst_type (orig_type, 0);
+ 
+     case PLUS_EXPR:
+     case MINUS_EXPR:
+       op0 = TREE_OPERAND (expr, 0);
+       op1 = TREE_OPERAND (expr, 1);
+ 
+       op0 = strip_offset (op0, false, &off0);
+       op1 = strip_offset (op1, false, &off1);
+ 
+       *offset = (code == PLUS_EXPR ? off0 + off1 : off0 - off1);
+       if (op0 != TREE_OPERAND (expr, 0)
+ 	  || op1 != TREE_OPERAND (expr, 1))
+ 	{
+ 	  if (zero_p (op1))
+ 	    expr = op0;
+ 	  else if (zero_p (op0))
+ 	    {
+ 	      if (code == PLUS_EXPR)
+ 		expr = op1;
+ 	      else
+ 		expr = build1 (NEGATE_EXPR, type, op1);
+ 	    }
+ 	  else
+ 	    expr = build2 (code, type, op0, op1);
+ 	}
+ 
+       return fold_convert (orig_type, expr);
+ 
+     case ARRAY_REF:
+       if (!inside_addr)
+ 	return fold_convert (orig_type, expr);
+ 
+       step = array_ref_element_size (expr);
+       if (!cst_and_fits_in_hwi (step))
+ 	break;
+ 
+       st = int_cst_value (step);
+       op1 = TREE_OPERAND (expr, 1);
+       op1 = strip_offset (op1, false, &off1);
+       *offset = off1 * st;
+       break;
+ 
+     case COMPONENT_REF:
+       if (!inside_addr)
+ 	return fold_convert (orig_type, expr);
+       break;
+ 
+     case ADDR_EXPR:
+       inside_addr = true;
+       break;
+ 
+     default:
+       return fold_convert (orig_type, expr);
+     }
+ 
+   /* Default handling of expressions for that we want to recurse into
+      the first operand.  */
+   op0 = TREE_OPERAND (expr, 0);
+   op0 = strip_offset (op0, inside_addr, &off0);
+   *offset += off0;
+ 
+   if (op0 != TREE_OPERAND (expr, 0)
+       || (op1 && op1 != TREE_OPERAND (expr, 1)))
+     {
+       expr = copy_node (expr);
+       TREE_OPERAND (expr, 0) = op0;
+       if (op1)
+ 	TREE_OPERAND (expr, 1) = op1;
+     }
+ 
+   return fold_convert (orig_type, expr);
+ }
+ 
  /* Adds a candidate BASE + STEP * i.  Important field is set to IMPORTANT and
     position to POS.  If USE is not NULL, the candidate is set as related to
     it.  If both BASE and STEP are NULL, we add a pseudocandidate for the
*************** static void
*** 1900,1906 ****
  add_address_candidates (struct ivopts_data *data,
  			struct iv *iv, struct iv_use *use)
  {
!   tree base, abase, tmp, *act;
  
    /* First, the trivial choices.  */
    add_iv_value_candidates (data, iv, use);
--- 2001,2008 ----
  add_address_candidates (struct ivopts_data *data,
  			struct iv *iv, struct iv_use *use)
  {
!   tree base, abase;
!   unsigned HOST_WIDE_INT offset;
  
    /* First, the trivial choices.  */
    add_iv_value_candidates (data, iv, use);
*************** add_address_candidates (struct ivopts_da
*** 1929,1954 ****
  
    /* Third, try removing the constant offset.  */
    abase = iv->base;
!   while (TREE_CODE (abase) == PLUS_EXPR
! 	 && TREE_CODE (TREE_OPERAND (abase, 1)) != INTEGER_CST)
!     abase = TREE_OPERAND (abase, 0);
!   /* We found the offset, so make the copy of the non-shared part and
!      remove it.  */
!   if (TREE_CODE (abase) == PLUS_EXPR)
!     {
!       tmp = iv->base;
!       act = &base;
! 
!       for (tmp = iv->base; tmp != abase; tmp = TREE_OPERAND (tmp, 0))
! 	{
! 	  *act = build2 (PLUS_EXPR, TREE_TYPE (tmp),
! 			 NULL_TREE, TREE_OPERAND (tmp, 1));
! 	  act = &TREE_OPERAND (*act, 0);
! 	}
!       *act = TREE_OPERAND (tmp, 0);
! 
!       add_candidate (data, base, iv->step, false, use);
!     }
  }
  
  /* Possibly adds pseudocandidate for replacing the final value of USE by
--- 2031,2039 ----
  
    /* Third, try removing the constant offset.  */
    abase = iv->base;
!   base = strip_offset (abase, false, &offset);
!   if (offset)
!     add_candidate (data, base, iv->step, false, use);
  }
  
  /* Possibly adds pseudocandidate for replacing the final value of USE by
*************** get_computation (struct loop *loop, stru
*** 2415,2467 ****
    return get_computation_at (loop, use, cand, use->stmt);
  }
  
- /* Strips constant offsets from EXPR and adds them to OFFSET.  */
- 
- static void
- strip_offset (tree *expr, unsigned HOST_WIDE_INT *offset)
- {
-   tree op0, op1;
-   enum tree_code code;
-   
-   while (1)
-     {
-       if (cst_and_fits_in_hwi (*expr))
- 	{
- 	  *offset += int_cst_value (*expr);
- 	  *expr = integer_zero_node;
- 	  return;
- 	}
- 
-       code = TREE_CODE (*expr);
-      
-       if (code != PLUS_EXPR && code != MINUS_EXPR)
- 	return;
- 
-       op0 = TREE_OPERAND (*expr, 0);
-       op1 = TREE_OPERAND (*expr, 1);
- 
-       if (cst_and_fits_in_hwi (op1))
- 	{
- 	  if (code == PLUS_EXPR)
- 	    *offset += int_cst_value (op1);
- 	  else
- 	    *offset -= int_cst_value (op1);
- 
- 	  *expr = op0;
- 	  continue;
- 	}
- 
-       if (code != PLUS_EXPR)
- 	return;
- 
-       if (!cst_and_fits_in_hwi (op0))
- 	return;
- 
-       *offset += int_cst_value (op0);
-       *expr = op1;
-     }
- }
- 
  /* Returns cost of addition in MODE.  */
  
  static unsigned
--- 2500,2505 ----
*************** difference_cost (struct ivopts_data *dat
*** 2963,2973 ****
  {
    unsigned cost;
    enum machine_mode mode = TYPE_MODE (TREE_TYPE (e1));
  
!   strip_offset (&e1, offset);
!   *offset = -*offset;
!   strip_offset (&e2, offset);
!   *offset = -*offset;
  
    if (TREE_CODE (e1) == ADDR_EXPR)
      return ptr_difference_cost (data, e1, e2, symbol_present, var_present, offset,
--- 3001,3011 ----
  {
    unsigned cost;
    enum machine_mode mode = TYPE_MODE (TREE_TYPE (e1));
+   unsigned HOST_WIDE_INT off1, off2;
  
!   e1 = strip_offset (e1, false, &off1);
!   e2 = strip_offset (e2, false, &off2);
!   *offset += off1 - off2;
  
    if (TREE_CODE (e1) == ADDR_EXPR)
      return ptr_difference_cost (data, e1, e2, symbol_present, var_present, offset,


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