public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PR 58958: wrong aliasing info
@ 2013-11-01 22:39 Marc Glisse
  2013-11-04 10:55 ` Richard Biener
  0 siblings, 1 reply; 12+ messages in thread
From: Marc Glisse @ 2013-11-01 22:39 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: TEXT/PLAIN, Size: 697 bytes --]

Hello,

the issue was described in the PR and the message linked from there. 
ao_ref_init_from_ptr_and_size calls get_ref_base_and_extent, which may 
detect an array_ref of variable index, but ao_ref_init_from_ptr_and_size 
never learns of it and uses the offset+size as if they were meaningful.

Bootstrap+testsuite on x86_64-unknown-linux-gnu.

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

 	PR tree-optimization/
gcc/
 	* tree-dfa.h (get_addr_base_and_unit_offset_1): Add error reporting.
 	* tree-ssa-alias.c (ao_ref_init_from_ptr_and_size): Use
 	get_addr_base_and_unit_offset_1 instead of get_ref_base_and_extent.

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

-- 
Marc Glisse

[-- Attachment #2: Type: TEXT/PLAIN, Size: 7973 bytes --]

Index: gcc/testsuite/gcc.dg/tree-ssa/pr58958.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/pr58958.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr58958.c	(working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+double a[10];
+int f(int n){
+  a[3]=9;
+  __builtin_memset(&a[n],3,sizeof(double));
+  return a[3]==9;
+}
+
+/* { dg-final { scan-tree-dump " == 9" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: gcc/testsuite/gcc.dg/tree-ssa/pr58958.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-dfa.h
===================================================================
--- gcc/tree-dfa.h	(revision 204302)
+++ gcc/tree-dfa.h	(working copy)
@@ -30,65 +30,70 @@ extern tree ssa_default_def (struct func
 extern void set_ssa_default_def (struct function *, tree, tree);
 extern tree get_or_create_ssa_default_def (struct function *, tree);
 extern tree get_ref_base_and_extent (tree, HOST_WIDE_INT *,
 				     HOST_WIDE_INT *, HOST_WIDE_INT *);
 extern tree get_addr_base_and_unit_offset (tree, HOST_WIDE_INT *);
 extern bool stmt_references_abnormal_ssa_name (gimple);
 extern void dump_enumerated_decls (FILE *, int);
 
 /* Returns the base object and a constant BITS_PER_UNIT offset in *POFFSET that
    denotes the starting address of the memory access EXP.
-   Returns NULL_TREE if the offset is not constant or any component
-   is not BITS_PER_UNIT-aligned.
+   If the offset is not constant or any component is not BITS_PER_UNIT-aligned,
+   sets *SAFE to false or returns NULL_TREE if SAFE is NULL.
    VALUEIZE if non-NULL is used to valueize SSA names.  It should return
    its argument or a constant if the argument is known to be constant.  */
 /* ??? This is a static inline here to avoid the overhead of the indirect calls
    to VALUEIZE.  But is this overhead really that significant?  And should we
    perhaps just rely on WHOPR to specialize the function?  */
 
 static inline tree
 get_addr_base_and_unit_offset_1 (tree exp, HOST_WIDE_INT *poffset,
-				 tree (*valueize) (tree))
+				 tree (*valueize) (tree), bool *safe = NULL)
 {
   HOST_WIDE_INT byte_offset = 0;
+  bool issafe = true;
 
   /* Compute cumulative byte-offset for nested component-refs and array-refs,
      and find the ultimate containing object.  */
   while (1)
     {
       switch (TREE_CODE (exp))
 	{
 	case BIT_FIELD_REF:
 	  {
 	    HOST_WIDE_INT this_off = TREE_INT_CST_LOW (TREE_OPERAND (exp, 2));
 	    if (this_off % BITS_PER_UNIT)
-	      return NULL_TREE;
-	    byte_offset += this_off / BITS_PER_UNIT;
+	      issafe = false;
+	    else
+	      byte_offset += this_off / BITS_PER_UNIT;
 	  }
 	  break;
 
 	case COMPONENT_REF:
 	  {
 	    tree field = TREE_OPERAND (exp, 1);
 	    tree this_offset = component_ref_field_offset (exp);
 	    HOST_WIDE_INT hthis_offset;
 
 	    if (!this_offset
 		|| TREE_CODE (this_offset) != INTEGER_CST
 		|| (TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field))
 		    % BITS_PER_UNIT))
-	      return NULL_TREE;
-
-	    hthis_offset = TREE_INT_CST_LOW (this_offset);
-	    hthis_offset += (TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field))
-			     / BITS_PER_UNIT);
-	    byte_offset += hthis_offset;
+	      issafe = false;
+	    else
+	      {
+		hthis_offset = TREE_INT_CST_LOW (this_offset);
+		hthis_offset +=
+		  (TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field))
+		   / BITS_PER_UNIT);
+		byte_offset += hthis_offset;
+	      }
 	  }
 	  break;
 
 	case ARRAY_REF:
 	case ARRAY_RANGE_REF:
 	  {
 	    tree index = TREE_OPERAND (exp, 1);
 	    tree low_bound, unit_size;
 
 	    if (valueize
@@ -102,21 +107,21 @@ get_addr_base_and_unit_offset_1 (tree ex
 		&& (unit_size = array_ref_element_size (exp),
 		    TREE_CODE (unit_size) == INTEGER_CST))
 	      {
 		HOST_WIDE_INT hindex = TREE_INT_CST_LOW (index);
 
 		hindex -= TREE_INT_CST_LOW (low_bound);
 		hindex *= TREE_INT_CST_LOW (unit_size);
 		byte_offset += hindex;
 	      }
 	    else
-	      return NULL_TREE;
+	      issafe = false;
 	  }
 	  break;
 
 	case REALPART_EXPR:
 	  break;
 
 	case IMAGPART_EXPR:
 	  byte_offset += TREE_INT_CST_LOW (TYPE_SIZE_UNIT (TREE_TYPE (exp)));
 	  break;
 
@@ -148,37 +153,48 @@ get_addr_base_and_unit_offset_1 (tree ex
 	  {
 	    tree base = TREE_OPERAND (exp, 0);
 	    if (valueize
 		&& TREE_CODE (base) == SSA_NAME)
 	      base = (*valueize) (base);
 
 	    /* Hand back the decl for MEM[&decl, off].  */
 	    if (TREE_CODE (base) == ADDR_EXPR)
 	      {
 		if (TMR_INDEX (exp) || TMR_INDEX2 (exp))
-		  return NULL_TREE;
-		if (!integer_zerop (TMR_OFFSET (exp)))
+		  issafe = false;
+		else if (!integer_zerop (TMR_OFFSET (exp)))
 		  {
 		    double_int off = mem_ref_offset (exp);
 		    gcc_assert (off.high == -1 || off.high == 0);
 		    byte_offset += off.to_shwi ();
 		  }
 		exp = TREE_OPERAND (base, 0);
 	      }
 	    goto done;
 	  }
 
 	default:
 	  goto done;
 	}
 
       exp = TREE_OPERAND (exp, 0);
     }
 done:
 
-  *poffset = byte_offset;
-  return exp;
+  if (issafe)
+    {
+      *poffset = byte_offset;
+      return exp;
+    }
+  else if (safe)
+    {
+      *safe = false;
+      *poffset = 0;
+      return exp;
+    }
+  else
+    return NULL_TREE;
 }
 
 
 
 #endif /* GCC_TREE_DFA_H */
Index: gcc/tree-ssa-alias.c
===================================================================
--- gcc/tree-ssa-alias.c	(revision 204302)
+++ gcc/tree-ssa-alias.c	(working copy)
@@ -559,49 +559,54 @@ ao_ref_alias_set (ao_ref *ref)
 }
 
 /* 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
    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;
+  HOST_WIDE_INT t, extra_offset = 0;
+  bool safe = true;
   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)
+	       && (t = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0)
 	{
 	  ptr = gimple_assign_rhs1 (stmt);
-	  extra_offset = BITS_PER_UNIT * t1;
+	  extra_offset = BITS_PER_UNIT * t;
 	}
     }
 
   if (TREE_CODE (ptr) == ADDR_EXPR)
-    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
-					 &ref->offset, &t1, &t2);
+    {
+      ref->base = get_addr_base_and_unit_offset_1 (TREE_OPERAND (ptr, 0),
+						   &t, 0, &safe);
+      ref->offset = BITS_PER_UNIT * t;
+    }
   else
     {
       ref->base = build2 (MEM_REF, char_type_node,
 			  ptr, null_pointer_node);
       ref->offset = 0;
     }
   ref->offset += extra_offset;
-  if (size
+  if (safe
+      && size
       && host_integerp (size, 0)
       && TREE_INT_CST_LOW (size) * BITS_PER_UNIT / BITS_PER_UNIT
 	 == TREE_INT_CST_LOW (size))
     ref->max_size = ref->size = TREE_INT_CST_LOW (size) * BITS_PER_UNIT;
   else
     ref->max_size = ref->size = -1;
   ref->ref_alias_set = 0;
   ref->base_alias_set = 0;
   ref->volatile_p = false;
 }

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PR 58958: wrong aliasing info
  2013-11-01 22:39 PR 58958: wrong aliasing info Marc Glisse
@ 2013-11-04 10:55 ` Richard Biener
  2013-11-04 11:10   ` Richard Biener
  2013-11-04 11:13   ` Marc Glisse
  0 siblings, 2 replies; 12+ messages in thread
From: Richard Biener @ 2013-11-04 10:55 UTC (permalink / raw)
  To: Marc Glisse; +Cc: GCC Patches

On Fri, Nov 1, 2013 at 11:39 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> the issue was described in the PR and the message linked from there.
> ao_ref_init_from_ptr_and_size calls get_ref_base_and_extent, which may
> detect an array_ref of variable index, but ao_ref_init_from_ptr_and_size
> never learns of it and uses the offset+size as if they were meaningful.

Well...  it certainly learns of it, but it chooses to ignore...

   if (TREE_CODE (ptr) == ADDR_EXPR)
-    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
-                                        &ref->offset, &t1, &t2);
+    {
+      ref->base = get_addr_base_and_unit_offset_1 (TREE_OPERAND (ptr, 0),
+                                                  &t, 0, &safe);
+      ref->offset = BITS_PER_UNIT * t;
+    }

safe == (t1 != -1 && t1 == t2)

note that ao_ref_init_from_ptr_and_size gets the size fed in as argument
so I fail to see why it matters at all ...?  That is, if you feed in a wrong
size then it's the callers error.

Richard.

> Bootstrap+testsuite on x86_64-unknown-linux-gnu.
>
> 2013-11-04  Marc Glisse  <marc.glisse@inria.fr>
>
>         PR tree-optimization/
> gcc/
>         * tree-dfa.h (get_addr_base_and_unit_offset_1): Add error reporting.
>         * tree-ssa-alias.c (ao_ref_init_from_ptr_and_size): Use
>         get_addr_base_and_unit_offset_1 instead of get_ref_base_and_extent.
>
> gcc/testsuite/
>         * gcc.dg/tree-ssa/pr58958.c: New file.
>
> --
> Marc Glisse
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr58958.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/pr58958.c     (revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr58958.c     (working copy)
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +double a[10];
> +int f(int n){
> +  a[3]=9;
> +  __builtin_memset(&a[n],3,sizeof(double));
> +  return a[3]==9;
> +}
> +
> +/* { dg-final { scan-tree-dump " == 9" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
>
> Property changes on: gcc/testsuite/gcc.dg/tree-ssa/pr58958.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-dfa.h
> ===================================================================
> --- gcc/tree-dfa.h      (revision 204302)
> +++ gcc/tree-dfa.h      (working copy)
> @@ -30,65 +30,70 @@ extern tree ssa_default_def (struct func
>  extern void set_ssa_default_def (struct function *, tree, tree);
>  extern tree get_or_create_ssa_default_def (struct function *, tree);
>  extern tree get_ref_base_and_extent (tree, HOST_WIDE_INT *,
>                                      HOST_WIDE_INT *, HOST_WIDE_INT *);
>  extern tree get_addr_base_and_unit_offset (tree, HOST_WIDE_INT *);
>  extern bool stmt_references_abnormal_ssa_name (gimple);
>  extern void dump_enumerated_decls (FILE *, int);
>
>  /* Returns the base object and a constant BITS_PER_UNIT offset in *POFFSET
> that
>     denotes the starting address of the memory access EXP.
> -   Returns NULL_TREE if the offset is not constant or any component
> -   is not BITS_PER_UNIT-aligned.
> +   If the offset is not constant or any component is not
> BITS_PER_UNIT-aligned,
> +   sets *SAFE to false or returns NULL_TREE if SAFE is NULL.
>     VALUEIZE if non-NULL is used to valueize SSA names.  It should return
>     its argument or a constant if the argument is known to be constant.  */
>  /* ??? This is a static inline here to avoid the overhead of the indirect
> calls
>     to VALUEIZE.  But is this overhead really that significant?  And should
> we
>     perhaps just rely on WHOPR to specialize the function?  */
>
>  static inline tree
>  get_addr_base_and_unit_offset_1 (tree exp, HOST_WIDE_INT *poffset,
> -                                tree (*valueize) (tree))
> +                                tree (*valueize) (tree), bool *safe = NULL)
>  {
>    HOST_WIDE_INT byte_offset = 0;
> +  bool issafe = true;
>
>    /* Compute cumulative byte-offset for nested component-refs and
> array-refs,
>       and find the ultimate containing object.  */
>    while (1)
>      {
>        switch (TREE_CODE (exp))
>         {
>         case BIT_FIELD_REF:
>           {
>             HOST_WIDE_INT this_off = TREE_INT_CST_LOW (TREE_OPERAND (exp,
> 2));
>             if (this_off % BITS_PER_UNIT)
> -             return NULL_TREE;
> -           byte_offset += this_off / BITS_PER_UNIT;
> +             issafe = false;
> +           else
> +             byte_offset += this_off / BITS_PER_UNIT;
>           }
>           break;
>
>         case COMPONENT_REF:
>           {
>             tree field = TREE_OPERAND (exp, 1);
>             tree this_offset = component_ref_field_offset (exp);
>             HOST_WIDE_INT hthis_offset;
>
>             if (!this_offset
>                 || TREE_CODE (this_offset) != INTEGER_CST
>                 || (TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field))
>                     % BITS_PER_UNIT))
> -             return NULL_TREE;
> -
> -           hthis_offset = TREE_INT_CST_LOW (this_offset);
> -           hthis_offset += (TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET
> (field))
> -                            / BITS_PER_UNIT);
> -           byte_offset += hthis_offset;
> +             issafe = false;
> +           else
> +             {
> +               hthis_offset = TREE_INT_CST_LOW (this_offset);
> +               hthis_offset +=
> +                 (TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field))
> +                  / BITS_PER_UNIT);
> +               byte_offset += hthis_offset;
> +             }
>           }
>           break;
>
>         case ARRAY_REF:
>         case ARRAY_RANGE_REF:
>           {
>             tree index = TREE_OPERAND (exp, 1);
>             tree low_bound, unit_size;
>
>             if (valueize
> @@ -102,21 +107,21 @@ get_addr_base_and_unit_offset_1 (tree ex
>                 && (unit_size = array_ref_element_size (exp),
>                     TREE_CODE (unit_size) == INTEGER_CST))
>               {
>                 HOST_WIDE_INT hindex = TREE_INT_CST_LOW (index);
>
>                 hindex -= TREE_INT_CST_LOW (low_bound);
>                 hindex *= TREE_INT_CST_LOW (unit_size);
>                 byte_offset += hindex;
>               }
>             else
> -             return NULL_TREE;
> +             issafe = false;
>           }
>           break;
>
>         case REALPART_EXPR:
>           break;
>
>         case IMAGPART_EXPR:
>           byte_offset += TREE_INT_CST_LOW (TYPE_SIZE_UNIT (TREE_TYPE
> (exp)));
>           break;
>
> @@ -148,37 +153,48 @@ get_addr_base_and_unit_offset_1 (tree ex
>           {
>             tree base = TREE_OPERAND (exp, 0);
>             if (valueize
>                 && TREE_CODE (base) == SSA_NAME)
>               base = (*valueize) (base);
>
>             /* Hand back the decl for MEM[&decl, off].  */
>             if (TREE_CODE (base) == ADDR_EXPR)
>               {
>                 if (TMR_INDEX (exp) || TMR_INDEX2 (exp))
> -                 return NULL_TREE;
> -               if (!integer_zerop (TMR_OFFSET (exp)))
> +                 issafe = false;
> +               else if (!integer_zerop (TMR_OFFSET (exp)))
>                   {
>                     double_int off = mem_ref_offset (exp);
>                     gcc_assert (off.high == -1 || off.high == 0);
>                     byte_offset += off.to_shwi ();
>                   }
>                 exp = TREE_OPERAND (base, 0);
>               }
>             goto done;
>           }
>
>         default:
>           goto done;
>         }
>
>        exp = TREE_OPERAND (exp, 0);
>      }
>  done:
>
> -  *poffset = byte_offset;
> -  return exp;
> +  if (issafe)
> +    {
> +      *poffset = byte_offset;
> +      return exp;
> +    }
> +  else if (safe)
> +    {
> +      *safe = false;
> +      *poffset = 0;
> +      return exp;
> +    }
> +  else
> +    return NULL_TREE;
>  }
>
>
>
>  #endif /* GCC_TREE_DFA_H */
> Index: gcc/tree-ssa-alias.c
> ===================================================================
> --- gcc/tree-ssa-alias.c        (revision 204302)
> +++ gcc/tree-ssa-alias.c        (working copy)
> @@ -559,49 +559,54 @@ ao_ref_alias_set (ao_ref *ref)
>  }
>
>  /* 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
>     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;
> +  HOST_WIDE_INT t, extra_offset = 0;
> +  bool safe = true;
>    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)
> +              && (t = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0)
>         {
>           ptr = gimple_assign_rhs1 (stmt);
> -         extra_offset = BITS_PER_UNIT * t1;
> +         extra_offset = BITS_PER_UNIT * t;
>         }
>      }
>
>    if (TREE_CODE (ptr) == ADDR_EXPR)
> -    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
> -                                        &ref->offset, &t1, &t2);
> +    {
> +      ref->base = get_addr_base_and_unit_offset_1 (TREE_OPERAND (ptr, 0),
> +                                                  &t, 0, &safe);
> +      ref->offset = BITS_PER_UNIT * t;
> +    }
>    else
>      {
>        ref->base = build2 (MEM_REF, char_type_node,
>                           ptr, null_pointer_node);
>        ref->offset = 0;
>      }
>    ref->offset += extra_offset;
> -  if (size
> +  if (safe
> +      && size
>        && host_integerp (size, 0)
>        && TREE_INT_CST_LOW (size) * BITS_PER_UNIT / BITS_PER_UNIT
>          == TREE_INT_CST_LOW (size))
>      ref->max_size = ref->size = TREE_INT_CST_LOW (size) * BITS_PER_UNIT;
>    else
>      ref->max_size = ref->size = -1;
>    ref->ref_alias_set = 0;
>    ref->base_alias_set = 0;
>    ref->volatile_p = false;
>  }
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PR 58958: wrong aliasing info
  2013-11-04 10:55 ` Richard Biener
@ 2013-11-04 11:10   ` Richard Biener
  2013-11-04 11:23     ` Marc Glisse
  2013-11-04 11:13   ` Marc Glisse
  1 sibling, 1 reply; 12+ messages in thread
From: Richard Biener @ 2013-11-04 11:10 UTC (permalink / raw)
  To: Marc Glisse; +Cc: GCC Patches

On Mon, Nov 4, 2013 at 11:55 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Fri, Nov 1, 2013 at 11:39 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> the issue was described in the PR and the message linked from there.
>> ao_ref_init_from_ptr_and_size calls get_ref_base_and_extent, which may
>> detect an array_ref of variable index, but ao_ref_init_from_ptr_and_size
>> never learns of it and uses the offset+size as if they were meaningful.
>
> Well...  it certainly learns of it, but it chooses to ignore...
>
>    if (TREE_CODE (ptr) == ADDR_EXPR)
> -    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
> -                                        &ref->offset, &t1, &t2);
> +    {
> +      ref->base = get_addr_base_and_unit_offset_1 (TREE_OPERAND (ptr, 0),
> +                                                  &t, 0, &safe);
> +      ref->offset = BITS_PER_UNIT * t;
> +    }
>
> safe == (t1 != -1 && t1 == t2)
>
> note that ao_ref_init_from_ptr_and_size gets the size fed in as argument
> so I fail to see why it matters at all ...?  That is, if you feed in a wrong
> size then it's the callers error.

I think one issue is that the above code uses get_ref_base_and_extent
on an address.  It should have used get_addr_base_and_unit_offset.

Richard.

> Richard.
>
>> Bootstrap+testsuite on x86_64-unknown-linux-gnu.
>>
>> 2013-11-04  Marc Glisse  <marc.glisse@inria.fr>
>>
>>         PR tree-optimization/
>> gcc/
>>         * tree-dfa.h (get_addr_base_and_unit_offset_1): Add error reporting.
>>         * tree-ssa-alias.c (ao_ref_init_from_ptr_and_size): Use
>>         get_addr_base_and_unit_offset_1 instead of get_ref_base_and_extent.
>>
>> gcc/testsuite/
>>         * gcc.dg/tree-ssa/pr58958.c: New file.
>>
>> --
>> Marc Glisse
>> Index: gcc/testsuite/gcc.dg/tree-ssa/pr58958.c
>> ===================================================================
>> --- gcc/testsuite/gcc.dg/tree-ssa/pr58958.c     (revision 0)
>> +++ gcc/testsuite/gcc.dg/tree-ssa/pr58958.c     (working copy)
>> @@ -0,0 +1,12 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fdump-tree-optimized" } */
>> +
>> +double a[10];
>> +int f(int n){
>> +  a[3]=9;
>> +  __builtin_memset(&a[n],3,sizeof(double));
>> +  return a[3]==9;
>> +}
>> +
>> +/* { dg-final { scan-tree-dump " == 9" "optimized" } } */
>> +/* { dg-final { cleanup-tree-dump "optimized" } } */
>>
>> Property changes on: gcc/testsuite/gcc.dg/tree-ssa/pr58958.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-dfa.h
>> ===================================================================
>> --- gcc/tree-dfa.h      (revision 204302)
>> +++ gcc/tree-dfa.h      (working copy)
>> @@ -30,65 +30,70 @@ extern tree ssa_default_def (struct func
>>  extern void set_ssa_default_def (struct function *, tree, tree);
>>  extern tree get_or_create_ssa_default_def (struct function *, tree);
>>  extern tree get_ref_base_and_extent (tree, HOST_WIDE_INT *,
>>                                      HOST_WIDE_INT *, HOST_WIDE_INT *);
>>  extern tree get_addr_base_and_unit_offset (tree, HOST_WIDE_INT *);
>>  extern bool stmt_references_abnormal_ssa_name (gimple);
>>  extern void dump_enumerated_decls (FILE *, int);
>>
>>  /* Returns the base object and a constant BITS_PER_UNIT offset in *POFFSET
>> that
>>     denotes the starting address of the memory access EXP.
>> -   Returns NULL_TREE if the offset is not constant or any component
>> -   is not BITS_PER_UNIT-aligned.
>> +   If the offset is not constant or any component is not
>> BITS_PER_UNIT-aligned,
>> +   sets *SAFE to false or returns NULL_TREE if SAFE is NULL.
>>     VALUEIZE if non-NULL is used to valueize SSA names.  It should return
>>     its argument or a constant if the argument is known to be constant.  */
>>  /* ??? This is a static inline here to avoid the overhead of the indirect
>> calls
>>     to VALUEIZE.  But is this overhead really that significant?  And should
>> we
>>     perhaps just rely on WHOPR to specialize the function?  */
>>
>>  static inline tree
>>  get_addr_base_and_unit_offset_1 (tree exp, HOST_WIDE_INT *poffset,
>> -                                tree (*valueize) (tree))
>> +                                tree (*valueize) (tree), bool *safe = NULL)
>>  {
>>    HOST_WIDE_INT byte_offset = 0;
>> +  bool issafe = true;
>>
>>    /* Compute cumulative byte-offset for nested component-refs and
>> array-refs,
>>       and find the ultimate containing object.  */
>>    while (1)
>>      {
>>        switch (TREE_CODE (exp))
>>         {
>>         case BIT_FIELD_REF:
>>           {
>>             HOST_WIDE_INT this_off = TREE_INT_CST_LOW (TREE_OPERAND (exp,
>> 2));
>>             if (this_off % BITS_PER_UNIT)
>> -             return NULL_TREE;
>> -           byte_offset += this_off / BITS_PER_UNIT;
>> +             issafe = false;
>> +           else
>> +             byte_offset += this_off / BITS_PER_UNIT;
>>           }
>>           break;
>>
>>         case COMPONENT_REF:
>>           {
>>             tree field = TREE_OPERAND (exp, 1);
>>             tree this_offset = component_ref_field_offset (exp);
>>             HOST_WIDE_INT hthis_offset;
>>
>>             if (!this_offset
>>                 || TREE_CODE (this_offset) != INTEGER_CST
>>                 || (TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field))
>>                     % BITS_PER_UNIT))
>> -             return NULL_TREE;
>> -
>> -           hthis_offset = TREE_INT_CST_LOW (this_offset);
>> -           hthis_offset += (TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET
>> (field))
>> -                            / BITS_PER_UNIT);
>> -           byte_offset += hthis_offset;
>> +             issafe = false;
>> +           else
>> +             {
>> +               hthis_offset = TREE_INT_CST_LOW (this_offset);
>> +               hthis_offset +=
>> +                 (TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field))
>> +                  / BITS_PER_UNIT);
>> +               byte_offset += hthis_offset;
>> +             }
>>           }
>>           break;
>>
>>         case ARRAY_REF:
>>         case ARRAY_RANGE_REF:
>>           {
>>             tree index = TREE_OPERAND (exp, 1);
>>             tree low_bound, unit_size;
>>
>>             if (valueize
>> @@ -102,21 +107,21 @@ get_addr_base_and_unit_offset_1 (tree ex
>>                 && (unit_size = array_ref_element_size (exp),
>>                     TREE_CODE (unit_size) == INTEGER_CST))
>>               {
>>                 HOST_WIDE_INT hindex = TREE_INT_CST_LOW (index);
>>
>>                 hindex -= TREE_INT_CST_LOW (low_bound);
>>                 hindex *= TREE_INT_CST_LOW (unit_size);
>>                 byte_offset += hindex;
>>               }
>>             else
>> -             return NULL_TREE;
>> +             issafe = false;
>>           }
>>           break;
>>
>>         case REALPART_EXPR:
>>           break;
>>
>>         case IMAGPART_EXPR:
>>           byte_offset += TREE_INT_CST_LOW (TYPE_SIZE_UNIT (TREE_TYPE
>> (exp)));
>>           break;
>>
>> @@ -148,37 +153,48 @@ get_addr_base_and_unit_offset_1 (tree ex
>>           {
>>             tree base = TREE_OPERAND (exp, 0);
>>             if (valueize
>>                 && TREE_CODE (base) == SSA_NAME)
>>               base = (*valueize) (base);
>>
>>             /* Hand back the decl for MEM[&decl, off].  */
>>             if (TREE_CODE (base) == ADDR_EXPR)
>>               {
>>                 if (TMR_INDEX (exp) || TMR_INDEX2 (exp))
>> -                 return NULL_TREE;
>> -               if (!integer_zerop (TMR_OFFSET (exp)))
>> +                 issafe = false;
>> +               else if (!integer_zerop (TMR_OFFSET (exp)))
>>                   {
>>                     double_int off = mem_ref_offset (exp);
>>                     gcc_assert (off.high == -1 || off.high == 0);
>>                     byte_offset += off.to_shwi ();
>>                   }
>>                 exp = TREE_OPERAND (base, 0);
>>               }
>>             goto done;
>>           }
>>
>>         default:
>>           goto done;
>>         }
>>
>>        exp = TREE_OPERAND (exp, 0);
>>      }
>>  done:
>>
>> -  *poffset = byte_offset;
>> -  return exp;
>> +  if (issafe)
>> +    {
>> +      *poffset = byte_offset;
>> +      return exp;
>> +    }
>> +  else if (safe)
>> +    {
>> +      *safe = false;
>> +      *poffset = 0;
>> +      return exp;
>> +    }
>> +  else
>> +    return NULL_TREE;
>>  }
>>
>>
>>
>>  #endif /* GCC_TREE_DFA_H */
>> Index: gcc/tree-ssa-alias.c
>> ===================================================================
>> --- gcc/tree-ssa-alias.c        (revision 204302)
>> +++ gcc/tree-ssa-alias.c        (working copy)
>> @@ -559,49 +559,54 @@ ao_ref_alias_set (ao_ref *ref)
>>  }
>>
>>  /* 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
>>     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;
>> +  HOST_WIDE_INT t, extra_offset = 0;
>> +  bool safe = true;
>>    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)
>> +              && (t = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0)
>>         {
>>           ptr = gimple_assign_rhs1 (stmt);
>> -         extra_offset = BITS_PER_UNIT * t1;
>> +         extra_offset = BITS_PER_UNIT * t;
>>         }
>>      }
>>
>>    if (TREE_CODE (ptr) == ADDR_EXPR)
>> -    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
>> -                                        &ref->offset, &t1, &t2);
>> +    {
>> +      ref->base = get_addr_base_and_unit_offset_1 (TREE_OPERAND (ptr, 0),
>> +                                                  &t, 0, &safe);
>> +      ref->offset = BITS_PER_UNIT * t;
>> +    }
>>    else
>>      {
>>        ref->base = build2 (MEM_REF, char_type_node,
>>                           ptr, null_pointer_node);
>>        ref->offset = 0;
>>      }
>>    ref->offset += extra_offset;
>> -  if (size
>> +  if (safe
>> +      && size
>>        && host_integerp (size, 0)
>>        && TREE_INT_CST_LOW (size) * BITS_PER_UNIT / BITS_PER_UNIT
>>          == TREE_INT_CST_LOW (size))
>>      ref->max_size = ref->size = TREE_INT_CST_LOW (size) * BITS_PER_UNIT;
>>    else
>>      ref->max_size = ref->size = -1;
>>    ref->ref_alias_set = 0;
>>    ref->base_alias_set = 0;
>>    ref->volatile_p = false;
>>  }
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PR 58958: wrong aliasing info
  2013-11-04 10:55 ` Richard Biener
  2013-11-04 11:10   ` Richard Biener
@ 2013-11-04 11:13   ` Marc Glisse
  1 sibling, 0 replies; 12+ messages in thread
From: Marc Glisse @ 2013-11-04 11:13 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On Mon, 4 Nov 2013, Richard Biener wrote:

> On Fri, Nov 1, 2013 at 11:39 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> the issue was described in the PR and the message linked from there.
>> ao_ref_init_from_ptr_and_size calls get_ref_base_and_extent, which may
>> detect an array_ref of variable index, but ao_ref_init_from_ptr_and_size
>> never learns of it and uses the offset+size as if they were meaningful.
>
> Well...  it certainly learns of it, but it chooses to ignore...
>
>   if (TREE_CODE (ptr) == ADDR_EXPR)
> -    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
> -                                        &ref->offset, &t1, &t2);
> +    {
> +      ref->base = get_addr_base_and_unit_offset_1 (TREE_OPERAND (ptr, 0),
> +                                                  &t, 0, &safe);
> +      ref->offset = BITS_PER_UNIT * t;
> +    }
>
> safe == (t1 != -1 && t1 == t2)

I'll try that... (I need to think whether that's sufficient to be safe)

> note that ao_ref_init_from_ptr_and_size gets the size fed in as argument
> so I fail to see why it matters at all ...?  That is, if you feed in a wrong
> size then it's the callers error.

The caller is feeding the right size. The issue is that 
get_ref_base_and_extent cannot determine the offset as a constant. 
Normally, get_ref_base_and_extent then gives you a safe combination of 
offset+maxsize to cover the whole decl. Here, we don't want to use the 
size determined by get_ref_base_and_extent, we know better, but that also 
means we have to handle the case where the offset couldn't be determined 
as a constant.

-- 
Marc Glisse

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PR 58958: wrong aliasing info
  2013-11-04 11:10   ` Richard Biener
@ 2013-11-04 11:23     ` Marc Glisse
  2013-11-04 12:00       ` Richard Biener
  0 siblings, 1 reply; 12+ messages in thread
From: Marc Glisse @ 2013-11-04 11:23 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On Mon, 4 Nov 2013, Richard Biener wrote:

> On Mon, Nov 4, 2013 at 11:55 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Fri, Nov 1, 2013 at 11:39 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>> Hello,
>>>
>>> the issue was described in the PR and the message linked from there.
>>> ao_ref_init_from_ptr_and_size calls get_ref_base_and_extent, which may
>>> detect an array_ref of variable index, but ao_ref_init_from_ptr_and_size
>>> never learns of it and uses the offset+size as if they were meaningful.
>>
>> Well...  it certainly learns of it, but it chooses to ignore...
>>
>>    if (TREE_CODE (ptr) == ADDR_EXPR)
>> -    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
>> -                                        &ref->offset, &t1, &t2);
>> +    {
>> +      ref->base = get_addr_base_and_unit_offset_1 (TREE_OPERAND (ptr, 0),
>> +                                                  &t, 0, &safe);
>> +      ref->offset = BITS_PER_UNIT * t;
>> +    }
>>
>> safe == (t1 != -1 && t1 == t2)
>>
>> note that ao_ref_init_from_ptr_and_size gets the size fed in as argument
>> so I fail to see why it matters at all ...?  That is, if you feed in a wrong
>> size then it's the callers error.
>
> I think one issue is that the above code uses get_ref_base_and_extent
> on an address.  It should have used get_addr_base_and_unit_offset.

Isn't that what my patch does? Except that get_addr_base_and_unit_offset 
often gives up and returns NULL_TREE, whereas I believe we still want a 
base even if we have trouble determining a constant offset, so I modified 
get_addr_base_and_unit_offset_1 a bit.

(not saying the result is pretty...)

-- 
Marc Glisse

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PR 58958: wrong aliasing info
  2013-11-04 11:23     ` Marc Glisse
@ 2013-11-04 12:00       ` Richard Biener
  2013-11-04 17:21         ` Marc Glisse
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2013-11-04 12:00 UTC (permalink / raw)
  To: Marc Glisse; +Cc: GCC Patches

On Mon, Nov 4, 2013 at 12:18 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Mon, 4 Nov 2013, Richard Biener wrote:
>
>> On Mon, Nov 4, 2013 at 11:55 AM, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>>
>>> On Fri, Nov 1, 2013 at 11:39 PM, Marc Glisse <marc.glisse@inria.fr>
>>> wrote:
>>>>
>>>> Hello,
>>>>
>>>> the issue was described in the PR and the message linked from there.
>>>> ao_ref_init_from_ptr_and_size calls get_ref_base_and_extent, which may
>>>> detect an array_ref of variable index, but ao_ref_init_from_ptr_and_size
>>>> never learns of it and uses the offset+size as if they were meaningful.
>>>
>>>
>>> Well...  it certainly learns of it, but it chooses to ignore...
>>>
>>>    if (TREE_CODE (ptr) == ADDR_EXPR)
>>> -    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
>>> -                                        &ref->offset, &t1, &t2);
>>> +    {
>>> +      ref->base = get_addr_base_and_unit_offset_1 (TREE_OPERAND (ptr,
>>> 0),
>>> +                                                  &t, 0, &safe);
>>> +      ref->offset = BITS_PER_UNIT * t;
>>> +    }
>>>
>>> safe == (t1 != -1 && t1 == t2)
>>>
>>> note that ao_ref_init_from_ptr_and_size gets the size fed in as argument
>>> so I fail to see why it matters at all ...?  That is, if you feed in a
>>> wrong
>>> size then it's the callers error.
>>
>>
>> I think one issue is that the above code uses get_ref_base_and_extent
>> on an address.  It should have used get_addr_base_and_unit_offset.
>
>
> Isn't that what my patch does? Except that get_addr_base_and_unit_offset
> often gives up and returns NULL_TREE, whereas I believe we still want a base
> even if we have trouble determining a constant offset, so I modified
> get_addr_base_and_unit_offset_1 a bit.
>
> (not saying the result is pretty...)

I don't think we want get_addr_base_and_unit_offset to return non-NULL
for a non-constant offset.  But yes, inside your patch is the correct fix,
so if you drop changing get_addr_base_and_unit_offset ...

Richard.

> --
> Marc Glisse

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PR 58958: wrong aliasing info
  2013-11-04 12:00       ` Richard Biener
@ 2013-11-04 17:21         ` Marc Glisse
  2013-11-04 20:24           ` Richard Biener
  0 siblings, 1 reply; 12+ messages in thread
From: Marc Glisse @ 2013-11-04 17:21 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On Mon, 4 Nov 2013, Richard Biener wrote:

> On Mon, Nov 4, 2013 at 12:18 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> On Mon, 4 Nov 2013, Richard Biener wrote:
>>
>>> On Mon, Nov 4, 2013 at 11:55 AM, Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>>
>>>> On Fri, Nov 1, 2013 at 11:39 PM, Marc Glisse <marc.glisse@inria.fr>
>>>> wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> the issue was described in the PR and the message linked from there.
>>>>> ao_ref_init_from_ptr_and_size calls get_ref_base_and_extent, which may
>>>>> detect an array_ref of variable index, but ao_ref_init_from_ptr_and_size
>>>>> never learns of it and uses the offset+size as if they were meaningful.
>>>>
>>>>
>>>> Well...  it certainly learns of it, but it chooses to ignore...
>>>>
>>>>    if (TREE_CODE (ptr) == ADDR_EXPR)
>>>> -    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
>>>> -                                        &ref->offset, &t1, &t2);
>>>> +    {
>>>> +      ref->base = get_addr_base_and_unit_offset_1 (TREE_OPERAND (ptr,
>>>> 0),
>>>> +                                                  &t, 0, &safe);
>>>> +      ref->offset = BITS_PER_UNIT * t;
>>>> +    }
>>>>
>>>> safe == (t1 != -1 && t1 == t2)
>>>>
>>>> note that ao_ref_init_from_ptr_and_size gets the size fed in as argument
>>>> so I fail to see why it matters at all ...?  That is, if you feed in a
>>>> wrong
>>>> size then it's the callers error.
>>>
>>>
>>> I think one issue is that the above code uses get_ref_base_and_extent
>>> on an address.  It should have used get_addr_base_and_unit_offset.
>>
>>
>> Isn't that what my patch does? Except that get_addr_base_and_unit_offset
>> often gives up and returns NULL_TREE, whereas I believe we still want a base
>> even if we have trouble determining a constant offset, so I modified
>> get_addr_base_and_unit_offset_1 a bit.
>>
>> (not saying the result is pretty...)
>
> I don't think we want get_addr_base_and_unit_offset to return non-NULL
> for a non-constant offset.  But yes, inside your patch is the correct fix,
> so if you drop changing get_addr_base_and_unit_offset ...

That means that in a number of cases, ao_ref_init_from_ptr_and_size will 
produce a meaningless ao_ref (both .ref and .base are 0). I expect that 
will cause regressions in code quality at least. Or did you mean something 
else?

get_inner_reference might be an option too (we have quite a few functions 
doing similar things).

-- 
Marc Glisse

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PR 58958: wrong aliasing info
  2013-11-04 17:21         ` Marc Glisse
@ 2013-11-04 20:24           ` Richard Biener
  2013-11-05 10:46             ` Marc Glisse
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2013-11-04 20:24 UTC (permalink / raw)
  To: Marc Glisse; +Cc: GCC Patches

Marc Glisse <marc.glisse@inria.fr> wrote:
>On Mon, 4 Nov 2013, Richard Biener wrote:
>
>> On Mon, Nov 4, 2013 at 12:18 PM, Marc Glisse <marc.glisse@inria.fr>
>wrote:
>>> On Mon, 4 Nov 2013, Richard Biener wrote:
>>>
>>>> On Mon, Nov 4, 2013 at 11:55 AM, Richard Biener
>>>> <richard.guenther@gmail.com> wrote:
>>>>>
>>>>> On Fri, Nov 1, 2013 at 11:39 PM, Marc Glisse
><marc.glisse@inria.fr>
>>>>> wrote:
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> the issue was described in the PR and the message linked from
>there.
>>>>>> ao_ref_init_from_ptr_and_size calls get_ref_base_and_extent,
>which may
>>>>>> detect an array_ref of variable index, but
>ao_ref_init_from_ptr_and_size
>>>>>> never learns of it and uses the offset+size as if they were
>meaningful.
>>>>>
>>>>>
>>>>> Well...  it certainly learns of it, but it chooses to ignore...
>>>>>
>>>>>    if (TREE_CODE (ptr) == ADDR_EXPR)
>>>>> -    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
>>>>> -                                        &ref->offset, &t1, &t2);
>>>>> +    {
>>>>> +      ref->base = get_addr_base_and_unit_offset_1 (TREE_OPERAND
>(ptr,
>>>>> 0),
>>>>> +                                                  &t, 0, &safe);
>>>>> +      ref->offset = BITS_PER_UNIT * t;
>>>>> +    }
>>>>>
>>>>> safe == (t1 != -1 && t1 == t2)
>>>>>
>>>>> note that ao_ref_init_from_ptr_and_size gets the size fed in as
>argument
>>>>> so I fail to see why it matters at all ...?  That is, if you feed
>in a
>>>>> wrong
>>>>> size then it's the callers error.
>>>>
>>>>
>>>> I think one issue is that the above code uses
>get_ref_base_and_extent
>>>> on an address.  It should have used get_addr_base_and_unit_offset.
>>>
>>>
>>> Isn't that what my patch does? Except that
>get_addr_base_and_unit_offset
>>> often gives up and returns NULL_TREE, whereas I believe we still
>want a base
>>> even if we have trouble determining a constant offset, so I modified
>>> get_addr_base_and_unit_offset_1 a bit.
>>>
>>> (not saying the result is pretty...)
>>
>> I don't think we want get_addr_base_and_unit_offset to return
>non-NULL
>> for a non-constant offset.  But yes, inside your patch is the correct
>fix,
>> so if you drop changing get_addr_base_and_unit_offset ...
>
>That means that in a number of cases, ao_ref_init_from_ptr_and_size
>will 
>produce a meaningless ao_ref (both .ref and .base are 0). I expect that
>
>will cause regressions in code quality at least. Or did you mean
>something 
>else?

Well, you cannot use the size argument unchanged for the null return case.  You could fallback to get_base_address and -1 size in that case.

Richard

>get_inner_reference might be an option too (we have quite a few
>functions 
>doing similar things).


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PR 58958: wrong aliasing info
  2013-11-04 20:24           ` Richard Biener
@ 2013-11-05 10:46             ` Marc Glisse
  2013-11-05 11:04               ` Jakub Jelinek
  2013-11-05 12:10               ` Richard Biener
  0 siblings, 2 replies; 12+ messages in thread
From: Marc Glisse @ 2013-11-05 10:46 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

[-- Attachment #1: Type: TEXT/PLAIN, Size: 647 bytes --]

On Mon, 4 Nov 2013, Richard Biener wrote:

> Well, you cannot use the size argument unchanged for the null return 
> case.  You could fallback to get_base_address and -1 size in that case.

Like this? Bootstrap+testsuite on x86_64-unknown-linux-gnu.
(I think I'll disable cilk for my future bootstraps: it takes forever and 
confuses contrib/compare_tests)

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

 	PR tree-optimization/58958
gcc/
         * tree-ssa-alias.c (ao_ref_init_from_ptr_and_size): Use
 	get_addr_base_and_unit_offset instead of get_ref_base_and_extent.

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

-- 
Marc Glisse

[-- Attachment #2: Type: TEXT/PLAIN, Size: 2943 bytes --]

Index: gcc/testsuite/gcc.dg/tree-ssa/pr58958.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/pr58958.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr58958.c	(working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+double a[10];
+int f(int n){
+  a[3]=9;
+  __builtin_memset(&a[n],3,sizeof(double));
+  return a[3]==9;
+}
+
+/* { dg-final { scan-tree-dump " == 9" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: gcc/testsuite/gcc.dg/tree-ssa/pr58958.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)
@@ -559,41 +559,50 @@ ao_ref_alias_set (ao_ref *ref)
 }
 
 /* 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
    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;
+  HOST_WIDE_INT t, 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)
+	       && (t = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0)
 	{
 	  ptr = gimple_assign_rhs1 (stmt);
-	  extra_offset = BITS_PER_UNIT * t1;
+	  extra_offset = BITS_PER_UNIT * t;
 	}
     }
 
   if (TREE_CODE (ptr) == ADDR_EXPR)
-    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
-					 &ref->offset, &t1, &t2);
+    {
+      ref->base = get_addr_base_and_unit_offset (TREE_OPERAND (ptr, 0), &t);
+      if (ref->base)
+	ref->offset = BITS_PER_UNIT * t;
+      else
+	{
+	  size = NULL_TREE;
+	  ref->offset = 0;
+	  ref->base = get_base_address (TREE_OPERAND (ptr, 0));
+	}
+    }
   else
     {
       ref->base = build2 (MEM_REF, char_type_node,
 			  ptr, null_pointer_node);
       ref->offset = 0;
     }
   ref->offset += extra_offset;
   if (size
       && host_integerp (size, 0)
       && TREE_INT_CST_LOW (size) * BITS_PER_UNIT / BITS_PER_UNIT

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PR 58958: wrong aliasing info
  2013-11-05 10:46             ` Marc Glisse
@ 2013-11-05 11:04               ` Jakub Jelinek
  2013-11-05 13:58                 ` Iyer, Balaji V
  2013-11-05 12:10               ` Richard Biener
  1 sibling, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2013-11-05 11:04 UTC (permalink / raw)
  To: Marc Glisse, Iyer, Balaji V; +Cc: Richard Biener, GCC Patches

On Tue, Nov 05, 2013 at 11:40:02AM +0100, Marc Glisse wrote:
> >Well, you cannot use the size argument unchanged for the null
> >return case.  You could fallback to get_base_address and -1 size
> >in that case.
> 
> Like this? Bootstrap+testsuite on x86_64-unknown-linux-gnu.
> (I think I'll disable cilk for my future bootstraps: it takes
> forever and confuses contrib/compare_tests)

Ah, I was wondering why my make check times (admittedly
--enable-checking=yes,rtl but I've done that for years) went up drastically
between Friday and Monday (from around 40 minutes to 70 minutes or more).

That is clearly highly undesirable.  Looking at
gcc.dg/cilk-plus/cilk-plus.exp
it seems every test is run 24 resp. 25 times, that is clearly way too much,
sure, we have torture kind of tests, but that is typically 6 or 8 times at
most, and it also depends on how expensive the tests are.
As the /AN/ tests were already preexisting, I guess we are talking about
the 9 new /CK/ dg-do run tests, so first of all, they must be very expensive
themselves, times 25:

dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus $ALWAYS_CFLAGS " " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O0 -fcilkplus $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O1 -fcilkplus $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O2 -ftree-vectorize -fcilkplus $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O3 -fcilkplus $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -g -fcilkplus $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -g -O0 -fcilkplus $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -g -O1 -fcilkplus $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -g -O2 -ftree-vectorize -fcilkplus $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -g -O3 -fcilkplus $ALWAYS_CFLAGS"  " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O3 -ftree-vectorize -fcilkplus -g $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -std=c99 $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -O0 -std=c99 $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -O1 -std=c99 $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -O2 -ftree-vectorize -std=c99 $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -O3 -std=c99 $ALWAYS_CFLAGS"  " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -g -std=c99 $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -g -O0 -std=c99 $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -g -O1 -std=c99 $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -g -O2 -ftree-vectorize -std=c99 $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -fcilkplus -g -O3 -std=c99 $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O3 -ftree-vectorize -std=c99 -g -fcilkplus $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O0 -flto -g -fcilkplus $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O2 -flto -g -fcilkplus $ALWAYS_CFLAGS" " "
dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O3  -flto -g -fcilkplus $ALWAYS_CFLAGS" " "

It doesn't make sense to test -g at all levels, just test it for one or two, it doesn't
make sense to test say no -O* vs. -O0, that is the same thing, or -O3 vs. -O3 -ftree-vectorize,
that is the same thing, testing -std=c99 vs. default is reasonable for compile time tests
if they particularly care for some reason, otherwise, just attach -std=c99 to one (doesn't matter
which one) variant.  So, please change this to at most 7 or so variants, and even then, if some of
the test is really expensive, use it as dg-do compile for all variants and dg-do run just for
one (say -O2 -ftree-vectorize or -O3 -g).

	Jakub

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: PR 58958: wrong aliasing info
  2013-11-05 10:46             ` Marc Glisse
  2013-11-05 11:04               ` Jakub Jelinek
@ 2013-11-05 12:10               ` Richard Biener
  1 sibling, 0 replies; 12+ messages in thread
From: Richard Biener @ 2013-11-05 12:10 UTC (permalink / raw)
  To: Marc Glisse; +Cc: GCC Patches

On Tue, Nov 5, 2013 at 11:40 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Mon, 4 Nov 2013, Richard Biener wrote:
>
>> Well, you cannot use the size argument unchanged for the null return case.
>> You could fallback to get_base_address and -1 size in that case.
>
>
> Like this? Bootstrap+testsuite on x86_64-unknown-linux-gnu.
> (I think I'll disable cilk for my future bootstraps: it takes forever and
> confuses contrib/compare_tests)

Yes, this is ok.

Thanks,
Richard.

> 2013-11-05  Marc Glisse  <marc.glisse@inria.fr>
>
>         PR tree-optimization/58958
> gcc/
>         * tree-ssa-alias.c (ao_ref_init_from_ptr_and_size): Use
>         get_addr_base_and_unit_offset instead of get_ref_base_and_extent.
>
>
> gcc/testsuite/
>         * gcc.dg/tree-ssa/pr58958.c: New file.
>
> --
> Marc Glisse
>
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr58958.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/pr58958.c     (revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr58958.c     (working copy)
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +double a[10];
> +int f(int n){
> +  a[3]=9;
> +  __builtin_memset(&a[n],3,sizeof(double));
> +  return a[3]==9;
> +}
> +
> +/* { dg-final { scan-tree-dump " == 9" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
>
> Property changes on: gcc/testsuite/gcc.dg/tree-ssa/pr58958.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)
> @@ -559,41 +559,50 @@ ao_ref_alias_set (ao_ref *ref)
>  }
>
>  /* 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
>     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;
> +  HOST_WIDE_INT t, 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)
> +              && (t = int_cst_value (gimple_assign_rhs2 (stmt))) >= 0)
>         {
>           ptr = gimple_assign_rhs1 (stmt);
> -         extra_offset = BITS_PER_UNIT * t1;
> +         extra_offset = BITS_PER_UNIT * t;
>         }
>      }
>
>    if (TREE_CODE (ptr) == ADDR_EXPR)
> -    ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
> -                                        &ref->offset, &t1, &t2);
> +    {
> +      ref->base = get_addr_base_and_unit_offset (TREE_OPERAND (ptr, 0),
> &t);
> +      if (ref->base)
> +       ref->offset = BITS_PER_UNIT * t;
> +      else
> +       {
> +         size = NULL_TREE;
> +         ref->offset = 0;
> +         ref->base = get_base_address (TREE_OPERAND (ptr, 0));
> +       }
> +    }
>    else
>      {
>        ref->base = build2 (MEM_REF, char_type_node,
>                           ptr, null_pointer_node);
>        ref->offset = 0;
>      }
>    ref->offset += extra_offset;
>    if (size
>        && host_integerp (size, 0)
>        && TREE_INT_CST_LOW (size) * BITS_PER_UNIT / BITS_PER_UNIT
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: PR 58958: wrong aliasing info
  2013-11-05 11:04               ` Jakub Jelinek
@ 2013-11-05 13:58                 ` Iyer, Balaji V
  0 siblings, 0 replies; 12+ messages in thread
From: Iyer, Balaji V @ 2013-11-05 13:58 UTC (permalink / raw)
  To: Jakub Jelinek, Marc Glisse; +Cc: Richard Biener, GCC Patches



> -----Original Message-----
> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: Tuesday, November 5, 2013 6:02 AM
> To: Marc Glisse; Iyer, Balaji V
> Cc: Richard Biener; GCC Patches
> Subject: Re: PR 58958: wrong aliasing info
> 
> On Tue, Nov 05, 2013 at 11:40:02AM +0100, Marc Glisse wrote:
> > >Well, you cannot use the size argument unchanged for the null return
> > >case.  You could fallback to get_base_address and -1 size in that
> > >case.
> >
> > Like this? Bootstrap+testsuite on x86_64-unknown-linux-gnu.
> > (I think I'll disable cilk for my future bootstraps: it takes forever
> > and confuses contrib/compare_tests)
> 
> Ah, I was wondering why my make check times (admittedly --enable-
> checking=yes,rtl but I've done that for years) went up drastically between
> Friday and Monday (from around 40 minutes to 70 minutes or more).
> 
> That is clearly highly undesirable.  Looking at gcc.dg/cilk-plus/cilk-plus.exp it
> seems every test is run 24 resp. 25 times, that is clearly way too much, sure,
> we have torture kind of tests, but that is typically 6 or 8 times at most, and it
> also depends on how expensive the tests are.
> As the /AN/ tests were already preexisting, I guess we are talking about the 9
> new /CK/ dg-do run tests, so first of all, they must be very expensive
> themselves, times 25:
> 
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -fcilkplus $ALWAYS_CFLAGS " " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O0 -fcilkplus $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O1 -fcilkplus $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O2 -ftree-vectorize -fcilkplus $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O3 -fcilkplus $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -g -fcilkplus $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -g -O0 -fcilkplus $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -g -O1 -fcilkplus $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -g -O2 -ftree-vectorize -fcilkplus $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -g -O3 -fcilkplus $ALWAYS_CFLAGS"  " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O3 -ftree-vectorize -fcilkplus -g $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -fcilkplus -std=c99 $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -fcilkplus -O0 -std=c99 $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -fcilkplus -O1 -std=c99 $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -fcilkplus -O2 -ftree-vectorize -std=c99 $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -fcilkplus -O3 -std=c99 $ALWAYS_CFLAGS"  " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -fcilkplus -g -std=c99 $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -fcilkplus -g -O0 -std=c99 $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -fcilkplus -g -O1 -std=c99 $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -fcilkplus -g -O2 -ftree-vectorize -std=c99 $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -fcilkplus -g -O3 -std=c99 $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O3 -ftree-vectorize -std=c99 -g -fcilkplus $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O0 -flto -g -fcilkplus $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O2 -flto -g -fcilkplus $ALWAYS_CFLAGS" " "
> dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O3  -flto -g -fcilkplus $ALWAYS_CFLAGS" " "
> 
> It doesn't make sense to test -g at all levels, just test it for one or two, it
> doesn't make sense to test say no -O* vs. -O0, that is the same thing, or -O3
> vs. -O3 -ftree-vectorize, that is the same thing, testing -std=c99 vs. default is
> reasonable for compile time tests if they particularly care for some reason,
> otherwise, just attach -std=c99 to one (doesn't matter which one) variant.
> So, please change this to at most 7 or so variants, and even then, if some of
> the test is really expensive, use it as dg-do compile for all variants and dg-do
> run just for one (say -O2 -ftree-vectorize or -O3 -g).
> 

Hi Jakub,
    Let me go through the testsuite script and I will remove some of the duplicates and send you a patch ASAP.

Thanks,

Balaji V. Iyer.

> 	Jakub

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-11-05 13:56 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-01 22:39 PR 58958: wrong aliasing info Marc Glisse
2013-11-04 10:55 ` Richard Biener
2013-11-04 11:10   ` Richard Biener
2013-11-04 11:23     ` Marc Glisse
2013-11-04 12:00       ` Richard Biener
2013-11-04 17:21         ` Marc Glisse
2013-11-04 20:24           ` Richard Biener
2013-11-05 10:46             ` Marc Glisse
2013-11-05 11:04               ` Jakub Jelinek
2013-11-05 13:58                 ` Iyer, Balaji V
2013-11-05 12:10               ` Richard Biener
2013-11-04 11:13   ` Marc Glisse

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).