Patchwork get_ref_base_and_extend based ctor folding

login
register
mail settings
Submitter Richard Guenther
Date Sept. 27, 2010, 12:21 p.m.
Message ID <alpine.LNX.2.00.1009271419150.8982@zhemvz.fhfr.qr>
Download mbox | patch
Permalink /patch/65837/
State New
Headers show

Comments

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

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;
 }