diff mbox

Aliasing: look through pointer's def stmt

Message ID alpine.DEB.2.02.1310311752160.5593@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse Oct. 31, 2013, 5:11 p.m. UTC 
On Wed, 30 Oct 2013, Richard Biener wrote:

>> --- gcc/tree-ssa-alias.c        (revision 204188)
>> +++ gcc/tree-ssa-alias.c        (working copy)
>> @@ -567,20 +567,29 @@ void
>>  ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, tree size)
>>  {
>>    HOST_WIDE_INT t1, t2;
>>    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)
>
> No need to restrict this to positive offsets I think.

Here is the follow-up patch to remove this restriction 
(bootstrap+testsuite on x86_64-unknown-linux-gnu).
I don't really know how safe it is.

I couldn't use host_integerp(,1) since it returns false for (size_t)(-1) 
on x86_64, and host_integerp(,0) seemed strange, but I could use it if you 
want.

2013-11-01  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.

Comments

Richard Biener Nov. 4, 2013, 1:49 p.m. UTC | #1 
On Thu, Oct 31, 2013 at 6:11 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Wed, 30 Oct 2013, Richard Biener wrote:
>
>>> --- gcc/tree-ssa-alias.c        (revision 204188)
>>> +++ gcc/tree-ssa-alias.c        (working copy)
>>> @@ -567,20 +567,29 @@ void
>>>  ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, tree size)
>>>  {
>>>    HOST_WIDE_INT t1, t2;
>>>    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)
>>
>>
>> No need to restrict this to positive offsets I think.
>
>
> Here is the follow-up patch to remove this restriction (bootstrap+testsuite
> on x86_64-unknown-linux-gnu).
> I don't really know how safe it is.
>
> I couldn't use host_integerp(,1) since it returns false for (size_t)(-1) on
> x86_64, and host_integerp(,0) seemed strange, but I could use it if you
> want.

Well, host_integer_p (, 0) looks correct to me.  But ...

> 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)

Otherwise ok.

Thanks,
Richard.

>  {
>    if (pos1 >= pos2
> -      && (size2 == (unsigned HOST_WIDE_INT)-1
> +      && (size2 == -1
>           || pos1 < (pos2 + size2)))
>      return true;
>    if (pos2 >= pos1
> -      && (size1 == (unsigned HOST_WIDE_INT)-1
> +      && (size1 == -1
>           || pos2 < (pos1 + size1)))
>      return true;
>
>    return false;
>  }



>
>
>  #endif /* TREE_SSA_ALIAS_H  */
>
Marc Glisse Nov. 4, 2013, 4:35 p.m. UTC | #2 
On Mon, 4 Nov 2013, Richard Biener wrote:

> Well, host_integer_p (, 0) looks correct to me.  But ...

Ok, I'll put it back.

>> 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?
Richard Biener Nov. 4, 2013, 8:25 p.m. UTC | #3 
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.
>
>>> 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.

Richard.
diff mbox

Patch

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 204267)
+++ 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 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)
 {
   if (pos1 >= pos2
-      && (size2 == (unsigned HOST_WIDE_INT)-1
+      && (size2 == -1
 	  || pos1 < (pos2 + size2)))
     return true;
   if (pos2 >= pos1
-      && (size1 == (unsigned HOST_WIDE_INT)-1
+      && (size1 == -1
 	  || pos2 < (pos1 + size1)))
     return true;
 
   return false;
 }
 
 
 
 #endif /* TREE_SSA_ALIAS_H  */