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: [RFA][PATCH] Minor fix to aliasing machinery


On Wed, Nov 6, 2013 at 1:19 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> [Discussion started in
> http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02472.html ]
>
>
> On Wed, 30 Oct 2013, Marc Glisse wrote:
>
>> On Wed, 30 Oct 2013, Richard Biener wrote:
>>
>>> Btw, get_addr_base_and_unit_offset may also return an offsetted
>>> MEM_REF (from &MEM [p_3, 17] for example).  As we are interested in
>>> pointers this could be handled by not requiring a memory reference
>>> but extracting the base address and offset, covering more cases.
>>
>>
>> I tried the attached patch, and it almost worked, except for one fortran
>> testcase (widechar_intrinsics_10.f90):
>
>
> Now that ao_ref_init_from_ptr_and_size has been fixed, the following patch
> passes bootstrap+testsuite (default languages) on x86_64-unknown-linux-gnu.
>
> 2013-11-06  Marc Glisse  <marc.glisse@inria.fr>
>             Jeff Law  <law@redhat.com>
>
> gcc/
>         * tree-ssa-alias.c (stmt_kills_ref_p_1): Use
>         ao_ref_init_from_ptr_and_size for builtins.
>
> gcc/testsuite/
>         * gcc.dg/tree-ssa/alias-27.c: New testcase.
>
> --
> Marc Glisse
> Index: gcc/testsuite/gcc.dg/tree-ssa/alias-27.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/alias-27.c    (revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/alias-27.c    (working copy)
> @@ -0,0 +1,11 @@
> +/* { 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" } } */
>
> Property changes on: gcc/testsuite/gcc.dg/tree-ssa/alias-27.c
> ___________________________________________________________________
> Added: svn:keywords
> ## -0,0 +1 ##
> +Author Date Id Revision URL
> \ No newline at end of property
> Added: svn:eol-style
> ## -0,0 +1 ##
> +native
> \ No newline at end of property
> Index: gcc/tree-ssa-alias.c
> ===================================================================
> --- gcc/tree-ssa-alias.c        (revision 204453)
> +++ gcc/tree-ssa-alias.c        (working copy)
> @@ -2001,23 +2001,24 @@ stmt_may_clobber_ref_p (gimple stmt, tre
>    return stmt_may_clobber_ref_p_1 (stmt, &r);
>  }
>
>  /* If STMT kills the memory reference REF return true, otherwise
>     return false.  */
>
>  static bool
>  stmt_kills_ref_p_1 (gimple stmt, ao_ref *ref)
>  {
>    /* For a must-alias check we need to be able to constrain
> -     the access properly.  */
> -  ao_ref_base (ref);
> -  if (ref->max_size == -1)
> +     the access properly.
> +     FIXME: except for BUILTIN_FREE.  */
> +  if (!ao_ref_base (ref)
> +      || ref->max_size == -1)
>      return false;
>
>    if (gimple_has_lhs (stmt)
>        && TREE_CODE (gimple_get_lhs (stmt)) != SSA_NAME
>        /* The assignment is not necessarily carried out if it can throw
>          and we can catch it in the current function where we could inspect
>          the previous value.
>          ???  We only need to care about the RHS throwing.  For aggregate
>          assignments or similar calls and non-call exceptions the LHS
>          might throw as well.  */
> @@ -2090,37 +2091,47 @@ stmt_kills_ref_p_1 (gimple stmt, ao_ref
>           case BUILT_IN_MEMPCPY:
>           case BUILT_IN_MEMMOVE:
>           case BUILT_IN_MEMSET:
>           case BUILT_IN_MEMCPY_CHK:
>           case BUILT_IN_MEMPCPY_CHK:
>           case BUILT_IN_MEMMOVE_CHK:
>           case BUILT_IN_MEMSET_CHK:
>             {
>               tree dest = gimple_call_arg (stmt, 0);
>               tree len = gimple_call_arg (stmt, 2);
> -             tree base = NULL_TREE;
> -             HOST_WIDE_INT offset = 0;
> +             tree rbase = ref->base;
> +             HOST_WIDE_INT roffset = ref->offset;
>               if (!host_integerp (len, 0))
>                 return false;
> -             if (TREE_CODE (dest) == ADDR_EXPR)
> -               base = get_addr_base_and_unit_offset (TREE_OPERAND (dest,
> 0),
> -                                                     &offset);
> -             else if (TREE_CODE (dest) == SSA_NAME)
> -               base = dest;
> -             if (base
> -                 && base == ao_ref_base (ref))
> +             ao_ref dref;
> +             ao_ref_init_from_ptr_and_size (&dref, dest, len);

What I dislike about this is that it can end up building a new tree node.
Not sure if that should block the patch though as it clearly allows to
simplify the code ...

> +             tree base = ao_ref_base (&dref);
> +             HOST_WIDE_INT offset = dref.offset;
> +             if (!base || dref.size == -1)
> +               return false;
> +             if (TREE_CODE (base) == MEM_REF)
> +               {
> +                 if (TREE_CODE (rbase) != MEM_REF)

why's that?  I think that just processing both bases separately
would work as well.

> +                   return false;
> +                 // Compare pointers.
> +                 offset += BITS_PER_UNIT
> +                           * TREE_INT_CST_LOW (TREE_OPERAND (base, 1));

Use mem_ref_offset (base).

> +                 roffset += BITS_PER_UNIT
> +                            * TREE_INT_CST_LOW (TREE_OPERAND (rbase, 1));

Likewise.

> +                 base = TREE_OPERAND (base, 0);
> +                 rbase = TREE_OPERAND (rbase, 0);

Both could be &decl here, so you want

          if (TREE_CODE (base) == ADDR_EXPR)
           base = TREE_OPERAND (base, 0);

Thanks,
Richard.

> +               }
> +             if (base == rbase)
>                 {
> -                 HOST_WIDE_INT size = TREE_INT_CST_LOW (len);
> -                 if (offset <= ref->offset / BITS_PER_UNIT
> -                     && (offset + size
> -                         >= ((ref->offset + ref->max_size + BITS_PER_UNIT -
> 1)
> -                             / BITS_PER_UNIT)))
> +                 HOST_WIDE_INT size = BITS_PER_UNIT * TREE_INT_CST_LOW
> (len);
> +                 if (offset <= roffset
> +                     && offset + size >= (roffset + ref->max_size))
>                     return true;
>                 }
>               break;
>             }
>
>           case BUILT_IN_VA_END:
>             {
>               tree ptr = gimple_call_arg (stmt, 0);
>               if (TREE_CODE (ptr) == ADDR_EXPR)
>                 {
>


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