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: [patch] Fix PR tree-optimization/46049 and tree-optimization/46052


On Sun, 17 Oct 2010, Ira Rosen wrote:

> 
> Hi,
> 
> With this patch we choose operands' type as the type of the vector created
> for them in case when the operands are invariant variables.
> In these PRs we have two different operations, so it doesn't seem
> appropriate to choose the type according to operation.
> 
> OK for trunk after bootstrap and testing complete on x86_64-suse-linux?

Well, I now looked at the problem myself.  The function seems to be
used to create vectors for operands which then get used to create
vectorized statements to produce their LHS (thus, much similar
to vect_get_vec_def_for_operand).  It thus does not make much sense
to look at STMT_VINFO_VECTYPE at all (that's the vector type of
the LHS of the stmt with the operands).  It should simply use
get_vectype_for_scalar_type always (or get_same_sized_vectype
with STMT_VINFO_VECTYPE as the vector type determining the size).

So either

Index: gcc/tree-vect-slp.c
===================================================================
--- gcc/tree-vect-slp.c (revision 165610)
+++ gcc/tree-vect-slp.c (working copy)
@@ -1899,14 +1899,7 @@ vect_get_constant_vectors (slp_tree slp_
   else
     constant_p = false;
 
-  /* For POINTER_PLUS_EXPR we use the type of the constant/invariant 
itself.
-     If OP is the first operand of POINTER_PLUS_EXPR, its type is the 
type of
-     the statement, so it's OK to use OP's type for both first and second
-     operands.  */
-  if (code == POINTER_PLUS_EXPR)
-    vector_type = get_vectype_for_scalar_type (TREE_TYPE (op));
-  else
-    vector_type = STMT_VINFO_VECTYPE (stmt_vinfo);
+  vector_type = get_vectype_for_scalar_type (TREE_TYPE (op));
 
   gcc_assert (vector_type);
   nunits = TYPE_VECTOR_SUBPARTS (vector_type);

or, to probably make AVX happier with float -> int conversion

Index: gcc/tree-vect-slp.c
===================================================================
--- gcc/tree-vect-slp.c (revision 165610)
+++ gcc/tree-vect-slp.c (working copy)
@@ -1899,14 +1899,8 @@ vect_get_constant_vectors (slp_tree slp_
   else
     constant_p = false;
 
-  /* For POINTER_PLUS_EXPR we use the type of the constant/invariant 
itself.
-     If OP is the first operand of POINTER_PLUS_EXPR, its type is the 
type of
-     the statement, so it's OK to use OP's type for both first and second
-     operands.  */
-  if (code == POINTER_PLUS_EXPR)
-    vector_type = get_vectype_for_scalar_type (TREE_TYPE (op));
-  else
-    vector_type = STMT_VINFO_VECTYPE (stmt_vinfo);
+  vector_type = get_same_sized_vectype (TREE_TYPE (op),
+                                       STMT_VINFO_VECTYPE (stmt_vinfo));
 
   gcc_assert (vector_type);
   nunits = TYPE_VECTOR_SUBPARTS (vector_type);

both variants pass the new testcases and the C vect testsuite for me.
Unless we start being less conservative with constraining vector
sizes with AVX there is probably no functional difference, but for
consistency I'd prefer the second variant.

Thanks,
Richard.


> Thanks,
> Ira
> 
> ChangeLog:
> 
> 	PR tree-optimization/46049
> 	PR tree-optimization/46052
> 	* tree-vect-slp.c (vect_get_constant_vectors): Use operands'
> 	type in case of invariant variables.
> 
> testsuite/ChangeLog:
> 
> 	PR tree-optimization/46049
> 	PR tree-optimization/46052
> 	* gcc.dg/vect/pr46052.c: New test.
> 	* gcc.dg/vect/pr46049.c: New test.
> 
> 
> Index: testsuite/gcc.dg/vect/pr46052.c
> ===================================================================
> --- testsuite/gcc.dg/vect/pr46052.c     (revision 0)
> +++ testsuite/gcc.dg/vect/pr46052.c     (revision 0)
> @@ -0,0 +1,33 @@
> +/* { dg-do compile } */
> +
> +int i;
> +int a[2];
> +
> +static inline char bar (void)
> +{
> +  return i ? i : 1;
> +}
> +
> +void foo (int n)
> +{
> +  while (n--)
> +    {
> +      a[0] ^= bar ();
> +      a[1] ^= bar ();
> +    }
> +}
> +
> +static inline char bar1 (void)
> +{
> +}
> +
> +void foo1 (int n)
> +{
> +  while (n--)
> +    {
> +      a[0] ^= bar1 ();
> +      a[1] ^= bar1 ();
> +    }
> +}
> +
> +/* { dg-final { cleanup-tree-dump "vect" } } */
> Index: testsuite/gcc.dg/vect/pr46049.c
> ===================================================================
> --- testsuite/gcc.dg/vect/pr46049.c     (revision 0)
> +++ testsuite/gcc.dg/vect/pr46049.c     (revision 0)
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +
> +typedef __INT16_TYPE__ int16_t;
> +typedef __INT32_TYPE__ int32_t;
> +
> +static inline int32_t bar (int16_t x, int16_t y)
> +{
> +  return x * y;
> +}
> +
> +void foo (int16_t i, int16_t *p, int16_t x)
> +{
> +  while (i--)
> +    {
> +      *p = bar (*p, x) >> 15;
> +      p++;
> +      *p = bar (*p, x) >> 15;
> +      p++;
> +    }
> +}
> +/* { dg-final { cleanup-tree-dump "vect" } } */
> Index: tree-vect-slp.c
> ===================================================================
> --- tree-vect-slp.c     (revision 165574)
> +++ tree-vect-slp.c     (working copy)
> @@ -1902,8 +1902,8 @@ vect_get_constant_vectors (slp_tree slp_
>    /* For POINTER_PLUS_EXPR we use the type of the constant/invariant
> itself.
>       If OP is the first operand of POINTER_PLUS_EXPR, its type is the type
> of
>       the statement, so it's OK to use OP's type for both first and second
> -     operands.  */
> -  if (code == POINTER_PLUS_EXPR)
> +     operands.  We also use the type of OP if it's an invariant variable.
> */
> +  if (code == POINTER_PLUS_EXPR || !constant_p)
>      vector_type = get_vectype_for_scalar_type (TREE_TYPE (op));
>    else
>      vector_type = STMT_VINFO_VECTYPE (stmt_vinfo);
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex


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