This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Improve stmt_kills_ref_p_1 (PR c++/34949)
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Jason Merrill <jason at redhat dot com>, gcc-patches at gcc dot gnu dot org
- Date: Tue, 2 Apr 2013 13:52:16 +0200 (CEST)
- Subject: Re: [PATCH] Improve stmt_kills_ref_p_1 (PR c++/34949)
- References: <20130402111159 dot GI20616 at tucnak dot redhat dot com>
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.