get_ref_base_and_extend based ctor folding

Submitted by Richard Guenther on Sept. 27, 2010, 12:21 p.m.

Details

Message ID alpine.LNX.2.00.1009271419150.8982@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Guenther Sept. 27, 2010, 12:21 p.m.
On Mon, 27 Sep 2010, Jan Hubicka wrote:

> > > ! 	  /* If the resulting bit-offset is constant, track it.  */
> > > ! 	  if ((low_bound = array_ref_low_bound (t),
> > > ! 	       host_integerp (low_bound, 0))
> > > ! 	      && (unit_size = array_ref_element_size (t),
> > > ! 		  host_integerp (unit_size, 1)))
> > > ! 	    {
> > > ! 	      offset = TREE_INT_CST_LOW (idx);
> > > ! 	      offset -= TREE_INT_CST_LOW (low_bound);
> > > ! 	      offset *= TREE_INT_CST_LOW (unit_size);
> > > ! 	      offset *= BITS_PER_UNIT;
> > > ! 
> > > ! 	      base = TREE_OPERAND (t, 0);
> > > ! 	      ctor = get_base_constructor (base, &ctr_offset);
> > > ! 	      if (ctr_offset)
> > > ! 		{
> > > ! 		  if (!host_integerp (ctr_offset, 1))
> > > ! 		    return NULL_TREE;
> > > ! 		  offset += TREE_INT_CST_LOW (ctr_offset) * BITS_PER_UNIT;
> > > ! 		}
> > > ! 	      /* Empty constructor.  Always fold to 0. */
> > > ! 	      if (ctor == error_mark_node)
> > > ! 		return build_zero_cst (TREE_TYPE (t));
> > > ! 	      /* Out of bound array access.  Value is undefined, but don't fold. */
> > > ! 	      if (offset < 0)
> > > ! 		return NULL_TREE;
> > > ! 	      /* We can not determine ctor.  */
> > > ! 	      if (!ctor)
> > > ! 		return NULL_TREE;
> > > ! 	      return fold_ctor_reference (TREE_TYPE (t), ctor, offset,
> > > ! 					  TREE_INT_CST_LOW (unit_size)
> > > ! 					  * BITS_PER_UNIT);
> > > ! 	    }
> > 
> > So this code is all just to handle first-level constant index?  Hm.
> 
> Yes, to handle a[i] where i is known to be 5.
> I tis ugly.  I think we can either move get_ref_base_and_extend to tree-ssa-ccp.c
> or just break it up to helper functions handling individual containers
> and feed it from tree-ssa-ccp with constant folded arguments.
> 
> > At least it looks odd that we are first throwing away the lower-bound
> > info here (not substituting constant SSA names ...) and then trying
> 
> low_bound is not thrown away, i tis used above for offset computation.
> We might want to constant fold it too, but it is not in SSA anyway?
> 
> > to reconstruct the proper index value using the low-bound in
> > fold_array_ctor_reference.  So it looks that at least from here
> > we could short-cut this possibly splitting fold_array_ctor_reference?
> 
> I am not sure I really like this idea - i.e. this way we have two
> independent functions solving two problems - get_ref_base_and_extend to convert
> reference into bit offset, size and type
> and fold_array_ctor_reference? taking bit offset, size and type and converting it
> to constructor value if known.  This way we can easily switch to target representation
> or so.  But we can deal with this incrementally.
> > 
> > 
> > But - I think the patch is ok as-is if you just move the 
> > if (max_size == -1 || max_size != size) check as suggested.
> > We can do the rest as followup.  This switching between
> > double-int and HWI looks fragile as well (yes, get_ref_base_and_extent
> > has the same issue ...).
> 
> Thanks a lot,
> I will update, retest and commit.
> 
> switching HWI and double-int is fragile indeed.  As I mentioned, I think HWI is safe
> for offset within memory representation of constructor, since we can just bail out
> for objects of size exeeding HWI range.  All temporary computations however should
> be double_int.
> 
> If you do have patch converting get_ref_base_and_extent to double_int, I can help
> with updating it for mainline.

I attached what is in my patches/ directory.  It doesn't change the
interfacing but just makes sure the result stays in HWI bounds.
And it needs updating to the post-MEM_REF state.

I'm not sure it's worth it in its current form as it may just increase
compile-time for no (practical) gain.

Richard.

Patch hide | download patch | download mbox

Index: gcc/tree-dfa.c
===================================================================
--- gcc/tree-dfa.c	(revision 163094)
+++ gcc/tree-dfa.c	(working copy)
@@ -708,7 +708,7 @@  get_ref_base_and_extent (tree exp, HOST_
   HOST_WIDE_INT bitsize = -1;
   HOST_WIDE_INT maxsize = -1;
   tree size_tree = NULL_TREE;
-  HOST_WIDE_INT bit_offset = 0;
+  double_int bit_offset = double_int_zero;
   bool seen_variable_array_ref = false;
 
   /* First get the final access size from just the outermost expression.  */
@@ -743,7 +743,9 @@  get_ref_base_and_extent (tree exp, HOST_
       switch (TREE_CODE (exp))
 	{
 	case BIT_FIELD_REF:
-	  bit_offset += TREE_INT_CST_LOW (TREE_OPERAND (exp, 2));
+	  bit_offset
+	    = double_int_add (bit_offset,
+			      tree_to_double_int (TREE_OPERAND (exp, 2)));
 	  break;
 
 	case COMPONENT_REF:
@@ -751,15 +753,24 @@  get_ref_base_and_extent (tree exp, HOST_
 	    tree field = TREE_OPERAND (exp, 1);
 	    tree this_offset = component_ref_field_offset (exp);
 
-	    if (this_offset
-		&& TREE_CODE (this_offset) == INTEGER_CST
-		&& host_integerp (this_offset, 0))
+	    /* We should not call get_ref_base_and_extent on
+	       not layouted fields.  */
+	    if (!this_offset)
+	      gcc_unreachable ();
+
+	    if (TREE_CODE (this_offset) == INTEGER_CST)
 	      {
-		HOST_WIDE_INT hthis_offset = TREE_INT_CST_LOW (this_offset);
-		hthis_offset *= BITS_PER_UNIT;
+		double_int hthis_offset = tree_to_double_int (this_offset);
+		hthis_offset
+		  = double_int_lshift (hthis_offset,
+				       BITS_PER_UNIT == 8
+				       ? 3 : exact_log2 (BITS_PER_UNIT),
+				       HOST_BITS_PER_DOUBLE_INT, true);
 		hthis_offset
-		  += TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field));
-		bit_offset += hthis_offset;
+		  = double_int_add (hthis_offset,
+				    tree_to_double_int
+				      (DECL_FIELD_BIT_OFFSET (field)));
+		bit_offset = double_int_add (bit_offset, hthis_offset);
 
 		/* If we had seen a variable array ref already and we just
 		   referenced the last field of a struct or a union member
@@ -777,11 +788,26 @@  get_ref_base_and_extent (tree exp, HOST_
 		      {
 			tree fsize = DECL_SIZE_UNIT (field);
 			tree ssize = TYPE_SIZE_UNIT (stype);
-			if (host_integerp (fsize, 0)
-			    && host_integerp (ssize, 0))
-			  maxsize += ((TREE_INT_CST_LOW (ssize)
-				       - TREE_INT_CST_LOW (fsize))
-				      * BITS_PER_UNIT - hthis_offset);
+			if (fsize && TREE_CODE (fsize) == INTEGER_CST
+			    && ssize && TREE_CODE (ssize) == INTEGER_CST)
+			  {
+			    double_int tem;
+			    tem = double_int_sub (tree_to_double_int (ssize),
+						  tree_to_double_int (fsize));
+			    tem = double_int_lshift (tem,
+						     BITS_PER_UNIT == 8
+						     ? 3 : exact_log2
+						             (BITS_PER_UNIT),
+						     HOST_BITS_PER_DOUBLE_INT,
+						     true);
+			    tem = double_int_sub (tem, hthis_offset);
+			    tem = double_int_add (shwi_to_double_int (maxsize),
+						  tem);
+			    if (double_int_fits_in_shwi_p (tem))
+			      maxsize = double_int_to_shwi (tem);
+			    else
+			      maxsize = -1;
+			  }
 			else
 			  maxsize = -1;
 		      }
@@ -793,8 +819,15 @@  get_ref_base_and_extent (tree exp, HOST_
 		/* We need to adjust maxsize to the whole structure bitsize.
 		   But we can subtract any constant offset seen so far,
 		   because that would get us out of the structure otherwise.  */
-		if (maxsize != -1 && csize && host_integerp (csize, 1))
-		  maxsize = TREE_INT_CST_LOW (csize) - bit_offset;
+		if (maxsize != -1 && csize && TREE_CODE (csize) == INTEGER_CST)
+		  {
+		    double_int tem
+		      = double_int_sub (tree_to_double_int (csize), bit_offset);
+		    if (double_int_fits_in_shwi_p (tem))
+		      maxsize = double_int_to_shwi (tem);
+		    else
+		      maxsize = -1;
+		  }
 		else
 		  maxsize = -1;
 	      }
@@ -809,18 +842,22 @@  get_ref_base_and_extent (tree exp, HOST_
 
 	    /* If the resulting bit-offset is constant, track it.  */
 	    if (TREE_CODE (index) == INTEGER_CST
-		&& host_integerp (index, 0)
 		&& (low_bound = array_ref_low_bound (exp),
-		    host_integerp (low_bound, 0))
+		    TREE_CODE (low_bound) == INTEGER_CST
 		&& (unit_size = array_ref_element_size (exp),
-		    host_integerp (unit_size, 1)))
+		    TREE_CODE (unit_size) == INTEGER_CST)))
 	      {
-		HOST_WIDE_INT hindex = TREE_INT_CST_LOW (index);
+		double_int hindex = tree_to_double_int (index);
 
-		hindex -= TREE_INT_CST_LOW (low_bound);
-		hindex *= TREE_INT_CST_LOW (unit_size);
-		hindex *= BITS_PER_UNIT;
-		bit_offset += hindex;
+		hindex = double_int_sub (hindex,
+					 tree_to_double_int (low_bound));
+		hindex = double_int_mul (hindex,
+					 tree_to_double_int (unit_size));
+		hindex = double_int_lshift (hindex,
+					    BITS_PER_UNIT == 8
+					    ? 3 : exact_log2 (BITS_PER_UNIT),
+					    HOST_BITS_PER_DOUBLE_INT, true);
+		bit_offset = double_int_add (bit_offset, hindex);
 
 		/* An array ref with a constant index up in the structure
 		   hierarchy will constrain the size of any variable array ref
@@ -833,8 +870,15 @@  get_ref_base_and_extent (tree exp, HOST_
 		/* We need to adjust maxsize to the whole array bitsize.
 		   But we can subtract any constant offset seen so far,
 		   because that would get us outside of the array otherwise.  */
-		if (maxsize != -1 && asize && host_integerp (asize, 1))
-		  maxsize = TREE_INT_CST_LOW (asize) - bit_offset;
+		if (maxsize != -1 && asize && TREE_CODE (asize) == INTEGER_CST)
+		  {
+		    double_int tem
+		      = double_int_sub (tree_to_double_int (asize), bit_offset);
+		    if (double_int_fits_in_shwi_p (tem))
+		      maxsize = double_int_to_shwi (tem);
+		    else
+		      maxsize = -1;
+		  }
 		else
 		  maxsize = -1;
 
@@ -849,7 +893,8 @@  get_ref_base_and_extent (tree exp, HOST_
 	  break;
 
 	case IMAGPART_EXPR:
-	  bit_offset += bitsize;
+	  bit_offset = double_int_add (bit_offset,
+				       shwi_to_double_int (bitsize));
 	  break;
 
 	case VIEW_CONVERT_EXPR:
@@ -868,12 +913,8 @@  get_ref_base_and_extent (tree exp, HOST_
 					   BITS_PER_UNIT == 8
 					   ? 3 : exact_log2 (BITS_PER_UNIT),
 					   HOST_BITS_PER_DOUBLE_INT, true);
-		  off = double_int_add (off, shwi_to_double_int (bit_offset));
-		  if (double_int_fits_in_shwi_p (off))
-		    {
-		      bit_offset = double_int_to_shwi (off);
-		      exp = TREE_OPERAND (TREE_OPERAND (exp, 0), 0);
-		    }
+		  bit_offset = double_int_add (off, bit_offset);
+		  exp = TREE_OPERAND (TREE_OPERAND (exp, 0), 0);
 		}
 	    }
 	  goto done;
@@ -904,22 +945,42 @@  get_ref_base_and_extent (tree exp, HOST_
       /* If maxsize is unknown adjust it according to the size of the
          base decl.  */
       if (maxsize == -1
-	  && host_integerp (DECL_SIZE (exp), 1))
-	maxsize = TREE_INT_CST_LOW (DECL_SIZE (exp)) - bit_offset;
+	  && DECL_SIZE (exp)
+	  && TREE_CODE (DECL_SIZE (exp)) == INTEGER_CST)
+	{
+	  double_int tem
+	    = double_int_sub (tree_to_double_int (DECL_SIZE (exp)), bit_offset);
+	  if (double_int_fits_in_shwi_p (tem))
+	    maxsize = double_int_to_shwi (tem);
+	}
     }
   else if (seen_variable_array_ref
 	   && maxsize != -1
-	   && (!host_integerp (TYPE_SIZE (TREE_TYPE (exp)), 1)
-	       || (bit_offset + maxsize
-		   == (signed) TREE_INT_CST_LOW (TYPE_SIZE (TREE_TYPE (exp))))))
+	   && (!TYPE_SIZE (TREE_TYPE (exp))
+	       || TREE_CODE (TYPE_SIZE (TREE_TYPE (exp))) != INTEGER_CST
+	       || double_int_equal_p
+	            (double_int_add (bit_offset, shwi_to_double_int (maxsize)),
+		     tree_to_double_int (TYPE_SIZE (TREE_TYPE (exp))))))
     maxsize = -1;
 
   /* ???  Due to negative offsets in ARRAY_REF we can end up with
      negative bit_offset here.  We might want to store a zero offset
      in this case.  */
-  *poffset = bit_offset;
+  /* ???  If bit_offset does not fit in a HOST_WIDE_INT give a
+     conservative answer.
+     ???  We really want to change get_ref_base_and_extent to return
+     a double_int for bit_offset instead.  */
+  if (!double_int_fits_in_shwi_p (bit_offset))
+    {
+      *poffset = 0;
+      *pmax_size = -1;
+    }
+  else
+    {
+      *poffset = double_int_to_shwi (bit_offset);
+      *pmax_size = maxsize;
+    }
   *psize = bitsize;
-  *pmax_size = maxsize;
 
   return exp;
 }