Aliasing: look through pointer's def stmt

On Mon, 4 Nov 2013, Richard Biener wrote:

> Marc Glisse <marc.glisse@inria.fr> wrote:
>> On Mon, 4 Nov 2013, Richard Biener wrote:
>>
>>> Well, host_integer_p (, 0) looks correct to me.  But ...
>>
>> Ok, I'll put it back.

Er, actually host_integerp(, 0) can't work, because arithmetic on pointers 
is done with unsigned integers and for x86_64 it would return false for 
(size_t)-4. So it would need to be host_integerp(, 1) instead. I still 
test TREE_CODE == INTEGER_CST in the current patch, let me know if you 
want host_integerp(, 1).

>>>> Index: gcc/tree-ssa-alias.h
>>>> ===================================================================
>>>> --- gcc/tree-ssa-alias.h        (revision 204267)
>>>> +++ gcc/tree-ssa-alias.h        (working copy)
>>>> @@ -139,30 +139,30 @@ extern void pt_solution_set_var (struct
>>>>
>>>>  extern void dump_pta_stats (FILE *);
>>>>
>>>>  extern GTY(()) struct pt_solution ipa_escaped_pt;
>>>>
>>>>  /* Return true, if the two ranges [POS1, SIZE1] and [POS2, SIZE2]
>>>>     overlap.  SIZE1 and/or SIZE2 can be (unsigned)-1 in which case
>> the
>>>>     range is open-ended.  Otherwise return false.  */
>>>>
>>>>  static inline bool
>>>> -ranges_overlap_p (unsigned HOST_WIDE_INT pos1,
>>>> -                 unsigned HOST_WIDE_INT size1,
>>>> -                 unsigned HOST_WIDE_INT pos2,
>>>> -                 unsigned HOST_WIDE_INT size2)
>>>> +ranges_overlap_p (HOST_WIDE_INT pos1,
>>>> +                 HOST_WIDE_INT size1,
>>>> +                 HOST_WIDE_INT pos2,
>>>> +                 HOST_WIDE_INT size2)
>>>
>>> I think size[12] should still be unsigned (we don't allow negative
>>> sizes but only the special value -1U which we special-case only to
>>> avoid pos + size to wrap)
>>
>> But all we do with size[12] is compare it to -1 and perform arithmetic
>> with pos[12]. At least for the second one, we need to cast size to a
>> signed type so the comparisons make sense (unless we decide to use a
>> double_int there). So I thought I would do the cast in the argument.
>> Otherwise, I'll start the function with:
>> HOST_WIDE_INT ssize1 = (HOST_WIDE_INT)size1;
>> and replace size1 with ssize1 through the function.
>>
>> Is the reason of keeping size[12] unsigned for documentation? Or am I
>> missing a reason why I may be breaking things?
>
> It is mostly documentation indeed.

Ok, then here it is (bootstrap+testsuite on x86_64-unknown-linux-gnu).

2013-11-05  Marc Glisse  <marc.glisse@inria.fr>

gcc/
 	* tree-ssa-alias.h (ranges_overlap_p): Handle negative offsets.
 	* tree-ssa-alias.c (ao_ref_init_from_ptr_and_size): Likewise.

gcc/testsuite/
 	* gcc.dg/tree-ssa/alias-26.c: New file.

On Tue, Nov 5, 2013 at 11:50 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Mon, 4 Nov 2013, Richard Biener wrote:
>
>> Marc Glisse <marc.glisse@inria.fr> wrote:
>>>
>>> On Mon, 4 Nov 2013, Richard Biener wrote:
>>>
>>>> Well, host_integer_p (, 0) looks correct to me.  But ...
>>>
>>>
>>> Ok, I'll put it back.
>
>
> Er, actually host_integerp(, 0) can't work, because arithmetic on pointers
> is done with unsigned integers and for x86_64 it would return false for
> (size_t)-4. So it would need to be host_integerp(, 1) instead. I still test
> TREE_CODE == INTEGER_CST in the current patch, let me know if you want
> host_integerp(, 1).

Hmm, indeed there isn't an exact match to what int_cst_value would
require.  Just testing INTEGER_CST should work, we know the
constant is of sizetype which needs to fit a HWI (in this case meaning
TREE_INT_CST_HIGH is either zero or -1).

>>>>> Index: gcc/tree-ssa-alias.h
>>>>> ===================================================================
>>>>> --- gcc/tree-ssa-alias.h        (revision 204267)
>>>>> +++ gcc/tree-ssa-alias.h        (working copy)
>>>>> @@ -139,30 +139,30 @@ extern void pt_solution_set_var (struct
>>>>>
>>>>>  extern void dump_pta_stats (FILE *);
>>>>>
>>>>>  extern GTY(()) struct pt_solution ipa_escaped_pt;
>>>>>
>>>>>  /* Return true, if the two ranges [POS1, SIZE1] and [POS2, SIZE2]
>>>>>     overlap.  SIZE1 and/or SIZE2 can be (unsigned)-1 in which case
>>>
>>> the
>>>>>
>>>>>     range is open-ended.  Otherwise return false.  */
>>>>>
>>>>>  static inline bool
>>>>> -ranges_overlap_p (unsigned HOST_WIDE_INT pos1,
>>>>> -                 unsigned HOST_WIDE_INT size1,
>>>>> -                 unsigned HOST_WIDE_INT pos2,
>>>>> -                 unsigned HOST_WIDE_INT size2)
>>>>> +ranges_overlap_p (HOST_WIDE_INT pos1,
>>>>> +                 HOST_WIDE_INT size1,
>>>>> +                 HOST_WIDE_INT pos2,
>>>>> +                 HOST_WIDE_INT size2)
>>>>
>>>>
>>>> I think size[12] should still be unsigned (we don't allow negative
>>>> sizes but only the special value -1U which we special-case only to
>>>> avoid pos + size to wrap)
>>>
>>>
>>> But all we do with size[12] is compare it to -1 and perform arithmetic
>>> with pos[12]. At least for the second one, we need to cast size to a
>>> signed type so the comparisons make sense (unless we decide to use a
>>> double_int there). So I thought I would do the cast in the argument.
>>> Otherwise, I'll start the function with:
>>> HOST_WIDE_INT ssize1 = (HOST_WIDE_INT)size1;
>>> and replace size1 with ssize1 through the function.
>>>
>>> Is the reason of keeping size[12] unsigned for documentation? Or am I
>>> missing a reason why I may be breaking things?
>>
>>
>> It is mostly documentation indeed.
>
>
> Ok, then here it is (bootstrap+testsuite on x86_64-unknown-linux-gnu).

Ok.

Thanks,
Richard.

> 2013-11-05  Marc Glisse  <marc.glisse@inria.fr>
>
>
> gcc/
>         * tree-ssa-alias.h (ranges_overlap_p): Handle negative offsets.
>         * tree-ssa-alias.c (ao_ref_init_from_ptr_and_size): Likewise.
>
> gcc/testsuite/
>         * gcc.dg/tree-ssa/alias-26.c: New file.
>
> --
> Marc Glisse
>
> Index: gcc/testsuite/gcc.dg/tree-ssa/alias-26.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/alias-26.c    (revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/alias-26.c    (working copy)
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +void f (const char *c, int *i)
> +{
> +  *i = 42;
> +  __builtin_memcpy (i - 1, c, sizeof (int));
> +  if (*i != 42) __builtin_abort();
> +}
> +
> +/* { dg-final { scan-tree-dump-not "abort" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
> +
>
> Property changes on: gcc/testsuite/gcc.dg/tree-ssa/alias-26.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 204381)
> +++ gcc/tree-ssa-alias.c        (working copy)
> @@ -552,42 +552,42 @@ ao_ref_base_alias_set (ao_ref *ref)
>  alias_set_type
>  ao_ref_alias_set (ao_ref *ref)
>  {
>    if (ref->ref_alias_set != -1)
>      return ref->ref_alias_set;
>    ref->ref_alias_set = get_alias_set (ref->ref);
>    return ref->ref_alias_set;
>  }
>
>  /* Init an alias-oracle reference representation from a gimple pointer
> -   PTR and a gimple size SIZE in bytes.  If SIZE is NULL_TREE the the
> +   PTR and a gimple size SIZE in bytes.  If SIZE is NULL_TREE then the
>     size is assumed to be unknown.  The access is assumed to be only
>     to or after of the pointer target, not before it.  */
>
>  void
>  ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, tree size)
>  {
>    HOST_WIDE_INT t1, t2, extra_offset = 0;
>    ref->ref = NULL_TREE;
>    if (TREE_CODE (ptr) == SSA_NAME)
>      {
>        gimple stmt = SSA_NAME_DEF_STMT (ptr);
>        if (gimple_assign_single_p (stmt)
>           && gimple_assign_rhs_code (stmt) == ADDR_EXPR)
>         ptr = gimple_assign_rhs1 (stmt);
>        else if (is_gimple_assign (stmt)
>                && gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR
> -              && host_integerp (gimple_assign_rhs2 (stmt), 0)
> -              && (t1 = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0)
> +              && TREE_CODE (gimple_assign_rhs2 (stmt)) == INTEGER_CST)
>         {
>           ptr = gimple_assign_rhs1 (stmt);
> -         extra_offset = BITS_PER_UNIT * t1;
> +         extra_offset = BITS_PER_UNIT
> +                        * int_cst_value (gimple_assign_rhs2 (stmt));
>         }
>      }
>
>    if (TREE_CODE (ptr) == ADDR_EXPR)
>      ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
>                                          &ref->offset, &t1, &t2);
>    else
>      {
>        ref->base = build2 (MEM_REF, char_type_node,
>                           ptr, null_pointer_node);
> Index: gcc/tree-ssa-alias.h
> ===================================================================
> --- gcc/tree-ssa-alias.h        (revision 204381)
> +++ gcc/tree-ssa-alias.h        (working copy)
> @@ -139,30 +139,30 @@ extern void pt_solution_set_var (struct
>
>  extern void dump_pta_stats (FILE *);
>
>  extern GTY(()) struct pt_solution ipa_escaped_pt;
>
>  /* Return true, if the two ranges [POS1, SIZE1] and [POS2, SIZE2]
>     overlap.  SIZE1 and/or SIZE2 can be (unsigned)-1 in which case the
>     range is open-ended.  Otherwise return false.  */
>
>  static inline bool
> -ranges_overlap_p (unsigned HOST_WIDE_INT pos1,
> +ranges_overlap_p (HOST_WIDE_INT pos1,
>                   unsigned HOST_WIDE_INT size1,
> -                 unsigned HOST_WIDE_INT pos2,
> +                 HOST_WIDE_INT pos2,
>                   unsigned HOST_WIDE_INT size2)
>  {
>    if (pos1 >= pos2
>        && (size2 == (unsigned HOST_WIDE_INT)-1
> -         || pos1 < (pos2 + size2)))
> +         || pos1 < (pos2 + (HOST_WIDE_INT) size2)))
>      return true;
>    if (pos2 >= pos1
>        && (size1 == (unsigned HOST_WIDE_INT)-1
> -         || pos2 < (pos1 + size1)))
> +         || pos2 < (pos1 + (HOST_WIDE_INT) size1)))
>      return true;
>
>    return false;
>  }
>
>
>
>  #endif /* TREE_SSA_ALIAS_H  */
>