diff mbox

Improve stmt_kills_ref_p_1 (PR c++/34949)

Message ID 20130402111159.GI20616@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek April 2, 2013, 11:11 a.m. UTC
Hi!

The patch I've just posted wasn't enough, because stmt_kills_ref_p_1
only did something if base == ref->base, but in the case of the dtors
base and ref->base are often MEM_REFs, which aren't equal, but they
just operand_equal_p.  And, for MEM_REFs, we don't even need to require
that the two MEM_REFs operand_equal_p, it is enough if the first operand
of both is the same, we don't need to care about either of the two types
(TBAA and MEM_REF's type), nor sizes, and can even handle different offset
arguments.  On IRC we've been talking with Richard about adding a predicate
function whether addresses are the same, but as that predicate would
necessarily require that the MEM_REF offset is the same constant (ignoring
the TBAA type on it), that would disallow handling different offsets,
so the function now performs all the checking inline.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2013-04-02  Jakub Jelinek  <jakub@redhat.com>

	PR c++/34949
	* tree-ssa-alias.c (stmt_kills_ref_p_1): If base != ref->base,
	call operand_equal_p on them or for MEM_REFs just compare
	first argument for equality and attempt to deal even with differing
	offsets.


	Jakub

Comments

Richard Biener April 2, 2013, 11:52 a.m. UTC | #1
On Tue, 2 Apr 2013, Jakub Jelinek wrote:

> Hi!
> 
> The patch I've just posted wasn't enough, because stmt_kills_ref_p_1
> only did something if base == ref->base, but in the case of the dtors
> base and ref->base are often MEM_REFs, which aren't equal, but they
> just operand_equal_p.  And, for MEM_REFs, we don't even need to require
> that the two MEM_REFs operand_equal_p, it is enough if the first operand
> of both is the same, we don't need to care about either of the two types
> (TBAA and MEM_REF's type), nor sizes, and can even handle different offset
> arguments.  On IRC we've been talking with Richard about adding a predicate
> function whether addresses are the same, but as that predicate would
> necessarily require that the MEM_REF offset is the same constant (ignoring
> the TBAA type on it), that would disallow handling different offsets,
> so the function now performs all the checking inline.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2013-04-02  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/34949
> 	* tree-ssa-alias.c (stmt_kills_ref_p_1): If base != ref->base,
> 	call operand_equal_p on them or for MEM_REFs just compare
> 	first argument for equality and attempt to deal even with differing
> 	offsets.
> 
> --- gcc/tree-ssa-alias.c.jj	2013-03-18 12:16:59.000000000 +0100
> +++ gcc/tree-ssa-alias.c	2013-03-27 17:09:28.230216081 +0100
> @@ -1870,20 +1870,51 @@ stmt_kills_ref_p_1 (gimple stmt, ao_ref
>        && !stmt_can_throw_internal (stmt))
>      {
>        tree base, lhs = gimple_get_lhs (stmt);
> -      HOST_WIDE_INT size, offset, max_size;
> +      HOST_WIDE_INT size, offset, max_size, ref_offset = ref->offset;
>        base = get_ref_base_and_extent (lhs, &offset, &size, &max_size);
>        /* We can get MEM[symbol: sZ, index: D.8862_1] here,
>  	 so base == ref->base does not always hold.  */
> -      if (base == ref->base)
> +      if (base != ref->base)
>  	{
> -	  /* For a must-alias check we need to be able to constrain
> -	     the access properly.  */
> -	  if (size != -1 && size == max_size)
> +	  /* If both base and ref->base are MEM_REFs, only compare the
> +	     first operand, and if the second operand isn't equal constant,
> +	     try to add the offsets into offset and ref_offset.  */
> +	  if (TREE_CODE (base) == MEM_REF && TREE_CODE (ref->base) == MEM_REF
> +	      && operand_equal_p (TREE_OPERAND (base, 0),
> +				  TREE_OPERAND (ref->base, 0), 0))

in all relevant cases operand zero of the MEM_REFs are SSA names
and thus can be compared for equality using == (exception are
integer constant pointers where the type could be theoretically
different).

>  	    {
> -	      if (offset <= ref->offset
> -		  && offset + size >= ref->offset + ref->max_size)
> -		return true;
> +	      if (!tree_int_cst_equal (TREE_OPERAND (base, 0),
> +				       TREE_OPERAND (ref->base, 0)))
> +		{
> +		  double_int off1 = mem_ref_offset (base);
> +		  off1 = off1.alshift (BITS_PER_UNIT == 8
> +				       ? 3 : exact_log2 (BITS_PER_UNIT),
> +				       HOST_BITS_PER_DOUBLE_INT);
> +		  off1 = off1 + double_int::from_shwi (offset);
> +		  double_int off2 = mem_ref_offset (ref->base);
> +		  off2 = off2.alshift (BITS_PER_UNIT == 8
> +				       ? 3 : exact_log2 (BITS_PER_UNIT),
> +				       HOST_BITS_PER_DOUBLE_INT);
> +		  off2 = off2 + double_int::from_shwi (ref_offset);
> +		  if (off1.fits_shwi () && off2.fits_shwi ())
> +		    {
> +		      offset = off1.to_shwi ();
> +		      ref_offset = off2.to_shwi ();
> +		    }
> +		  else
> +		    size = -1;
> +		}
>  	    }
> +	  else if (!operand_equal_p (base, ref->base, 0))
> +	    size = -1;

This does a redundant operand_equal_p check if both are MEM_REF
but with unequal pointers.  Did you get any case where the
2nd operand_equal_p would return true?  I can't think of any.
Thus, ok with

          else
            size = -1;

here.

Thanks,
Richard.
diff mbox

Patch

--- gcc/tree-ssa-alias.c.jj	2013-03-18 12:16:59.000000000 +0100
+++ gcc/tree-ssa-alias.c	2013-03-27 17:09:28.230216081 +0100
@@ -1870,20 +1870,51 @@  stmt_kills_ref_p_1 (gimple stmt, ao_ref
       && !stmt_can_throw_internal (stmt))
     {
       tree base, lhs = gimple_get_lhs (stmt);
-      HOST_WIDE_INT size, offset, max_size;
+      HOST_WIDE_INT size, offset, max_size, ref_offset = ref->offset;
       base = get_ref_base_and_extent (lhs, &offset, &size, &max_size);
       /* We can get MEM[symbol: sZ, index: D.8862_1] here,
 	 so base == ref->base does not always hold.  */
-      if (base == ref->base)
+      if (base != ref->base)
 	{
-	  /* For a must-alias check we need to be able to constrain
-	     the access properly.  */
-	  if (size != -1 && size == max_size)
+	  /* If both base and ref->base are MEM_REFs, only compare the
+	     first operand, and if the second operand isn't equal constant,
+	     try to add the offsets into offset and ref_offset.  */
+	  if (TREE_CODE (base) == MEM_REF && TREE_CODE (ref->base) == MEM_REF
+	      && operand_equal_p (TREE_OPERAND (base, 0),
+				  TREE_OPERAND (ref->base, 0), 0))
 	    {
-	      if (offset <= ref->offset
-		  && offset + size >= ref->offset + ref->max_size)
-		return true;
+	      if (!tree_int_cst_equal (TREE_OPERAND (base, 0),
+				       TREE_OPERAND (ref->base, 0)))
+		{
+		  double_int off1 = mem_ref_offset (base);
+		  off1 = off1.alshift (BITS_PER_UNIT == 8
+				       ? 3 : exact_log2 (BITS_PER_UNIT),
+				       HOST_BITS_PER_DOUBLE_INT);
+		  off1 = off1 + double_int::from_shwi (offset);
+		  double_int off2 = mem_ref_offset (ref->base);
+		  off2 = off2.alshift (BITS_PER_UNIT == 8
+				       ? 3 : exact_log2 (BITS_PER_UNIT),
+				       HOST_BITS_PER_DOUBLE_INT);
+		  off2 = off2 + double_int::from_shwi (ref_offset);
+		  if (off1.fits_shwi () && off2.fits_shwi ())
+		    {
+		      offset = off1.to_shwi ();
+		      ref_offset = off2.to_shwi ();
+		    }
+		  else
+		    size = -1;
+		}
 	    }
+	  else if (!operand_equal_p (base, ref->base, 0))
+	    size = -1;
+	}
+      /* For a must-alias check we need to be able to constrain
+	 the access properly.  */
+      if (size != -1 && size == max_size)
+	{
+	  if (offset <= ref_offset
+	      && offset + size >= ref_offset + ref->max_size)
+	    return true;
 	}
     }