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] Improve stmt_kills_ref_p_1 (PR c++/34949)


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.


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