Re: [RFA][PATCH] Minor fix to aliasing machinery

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Oct 29, 2013 at 10:36 PM, Jeff Law <law@redhat.com> wrote:
>
> Marc pointed out that the handling of various BUILT_IN_MEM* and
> BUILT_IN_STR* functions in tree-ssa-alias.c probably wasn't working as
> intended because the code wasn't prepared for a common return value from
> ao_ref_base, particularly returns of MEM_REFs.
>
> This patch fixes the code to handle the trivial case of returning a MEM_REF
> and adds a simple testcase.  There's probably a lot more that could be done
> here.
>
> Bootstrapped and regression tested on x86_64-unknown-linux-gnu.  Ok for the
> trunk?
>
> Thanks,
> Jeff
>
>         * tree-ssa-alias.c (stmt_kills_ref_p_1): Handle case where
>         ao_ref_base returns a MEM_REF.
>
>         * gcc.dg/tree-ssa/alias-26.c: New test.
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
> b/gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
> new file mode 100644
> index 0000000..b5625b8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-optimized" } */
> +
> +void f (long *p) {
> +  *p = 42;
> +  p[4] = 42;
> +  __builtin_memset (p, 0, 100);
> +}
> +
> +/* { dg-final { scan-tree-dump-not "= 42" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
> +
> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> index 4db83bd..5120e72 100644
> --- a/gcc/tree-ssa-alias.c
> +++ b/gcc/tree-ssa-alias.c
> @@ -2079,6 +2079,7 @@ stmt_kills_ref_p_1 (gimple stmt, ao_ref *ref)
>               tree dest = gimple_call_arg (stmt, 0);
>               tree len = gimple_call_arg (stmt, 2);
>               tree base = NULL_TREE;
> +             tree ref_base;
>               HOST_WIDE_INT offset = 0;
>               if (!host_integerp (len, 0))
>                 return false;
> @@ -2087,8 +2088,11 @@ stmt_kills_ref_p_1 (gimple stmt, ao_ref *ref)
>                                                       &offset);
>               else if (TREE_CODE (dest) == SSA_NAME)
>                 base = dest;
> +             ref_base = ao_ref_base (ref);
>               if (base
> -                 && base == ao_ref_base (ref))
> +                 && ((TREE_CODE (ref_base) == MEM_REF
> +                      && base == TREE_OPERAND (ref_base, 0))

That's not sufficient - ref_base may have an offset, so for correctness
you have to check that integer_zerop (TREE_OPERAND (ref_base, 0)).
But this now looks convoluted and somewhat backward, and still
does not catch all cases (including the def-stmt lookup recently
added to ao_ref_from_ptr_and_size).

Richard.

> +                     || ref_base == base))
>                 {
>                   HOST_WIDE_INT size = TREE_INT_CST_LOW (len);
>                   if (offset <= ref->offset / BITS_PER_UNIT
>