Patchwork Improving uniform_vector_p() function.

login
register
mail settings
Submitter Cong Hou
Date Oct. 1, 2013, 5:31 p.m.
Message ID <CAK=A3=3J=AnCYQSsYVEzoW0ETEov_UcLyjTT9SNGETGOn49Mhg@mail.gmail.com>
Download mbox | patch
Permalink /patch/279545/
State New
Headers show

Comments

Cong Hou - Oct. 1, 2013, 5:31 p.m.
The current uniform_vector_p() function only returns non-NULL when the
vector is directly a uniform vector. For example, for the following
gimple code:

vect_cst_.15_91 = {_9, _9, _9, _9, _9, _9, _9, _9};


The current implementation can only detect that {_9, _9, _9, _9, _9,
_9, _9, _9} is a uniform vector, but fails to recognize
vect_cst_.15_91 is also one. This simple patch searches through
assignment chains to find more uniform vectors.


thanks,
Cong
Xinliang David Li - Oct. 1, 2013, 9:37 p.m.
On Tue, Oct 1, 2013 at 10:31 AM, Cong Hou <congh@google.com> wrote:
> The current uniform_vector_p() function only returns non-NULL when the
> vector is directly a uniform vector. For example, for the following
> gimple code:
>
> vect_cst_.15_91 = {_9, _9, _9, _9, _9, _9, _9, _9};
>
>
> The current implementation can only detect that {_9, _9, _9, _9, _9,
> _9, _9, _9} is a uniform vector, but fails to recognize
> vect_cst_.15_91 is also one. This simple patch searches through
> assignment chains to find more uniform vectors.
>
>
> thanks,
> Cong
>
>
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 45c1667..b42f8a9 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2013-10-01  Cong Hou  <congh@google.com>
> +
> +       * tree.c: Improve the function uniform_vector_p() so that a
> +       vector assigned with a uniform vector is also treated as a
> +       uniform vector.
> +
> diff --git a/gcc/tree.c b/gcc/tree.c
> index 1c881e4..1d6d894 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -10297,6 +10297,17 @@ uniform_vector_p (const_tree vec)
>        return first;
>      }
>
> +  if (TREE_CODE (vec) == SSA_NAME)
> +    {
> +      gimple def = SSA_NAME_DEF_STMT (vec);
> +      if (gimple_code (def) == GIMPLE_ASSIGN)


do  this:

             if (is_gimple_assign (def) && gimple_assign_copy_p (def))

> +        {
> +          tree rhs = gimple_op (def, 1);
> +          if (VECTOR_TYPE_P (TREE_TYPE (rhs)))
> +            return uniform_vector_p (rhs);
> +        }
> +    }
> +
>    return NULL_TREE;
>  }

Do you have a test case showing what missed optimization this fix can enable ?

David
Cong Hou - Oct. 1, 2013, 9:44 p.m.
Actually I will introduce optimizations in the next patch. Currently
the function uniform_vector_p () is rarely used in GCC, but there are
certainly some optimization opportunities with the help of this
function.

For example, when we widen a vector with 8 identical element of short
type to two vectors of int type, GCC emits the following code:

  vect_cst_.15_91 = {_9, _9, _9, _9, _9, _9, _9, _9};
  vect__10.16_92 = [vec_unpack_lo_expr] vect_cst_.15_91;
  vect__10.16_93 = [vec_unpack_hi_expr] vect_cst_.15_91;

When vect_cst_.15_91 is a uniform vector, we know vect__10.16_92 and
vect__10.16_93 are identical so that we can remove the second
[vec_unpack_hi_expr] operation:

  vect_cst_.15_91 = {_9, _9, _9, _9, _9, _9, _9, _9};
  vect__10.16_92 = [vec_unpack_lo_expr] vect_cst_.15_91;
  vect__10.16_93 = vect__10.16_92;


thanks,
Cong


On Tue, Oct 1, 2013 at 2:37 PM, Xinliang David Li <davidxl@google.com> wrote:
> On Tue, Oct 1, 2013 at 10:31 AM, Cong Hou <congh@google.com> wrote:
>> The current uniform_vector_p() function only returns non-NULL when the
>> vector is directly a uniform vector. For example, for the following
>> gimple code:
>>
>> vect_cst_.15_91 = {_9, _9, _9, _9, _9, _9, _9, _9};
>>
>>
>> The current implementation can only detect that {_9, _9, _9, _9, _9,
>> _9, _9, _9} is a uniform vector, but fails to recognize
>> vect_cst_.15_91 is also one. This simple patch searches through
>> assignment chains to find more uniform vectors.
>>
>>
>> thanks,
>> Cong
>>
>>
>>
>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
>> index 45c1667..b42f8a9 100644
>> --- a/gcc/ChangeLog
>> +++ b/gcc/ChangeLog
>> @@ -1,3 +1,9 @@
>> +2013-10-01  Cong Hou  <congh@google.com>
>> +
>> +       * tree.c: Improve the function uniform_vector_p() so that a
>> +       vector assigned with a uniform vector is also treated as a
>> +       uniform vector.
>> +
>> diff --git a/gcc/tree.c b/gcc/tree.c
>> index 1c881e4..1d6d894 100644
>> --- a/gcc/tree.c
>> +++ b/gcc/tree.c
>> @@ -10297,6 +10297,17 @@ uniform_vector_p (const_tree vec)
>>        return first;
>>      }
>>
>> +  if (TREE_CODE (vec) == SSA_NAME)
>> +    {
>> +      gimple def = SSA_NAME_DEF_STMT (vec);
>> +      if (gimple_code (def) == GIMPLE_ASSIGN)
>
>
> do  this:
>
>              if (is_gimple_assign (def) && gimple_assign_copy_p (def))
>
>> +        {
>> +          tree rhs = gimple_op (def, 1);
>> +          if (VECTOR_TYPE_P (TREE_TYPE (rhs)))
>> +            return uniform_vector_p (rhs);
>> +        }
>> +    }
>> +
>>    return NULL_TREE;
>>  }
>
> Do you have a test case showing what missed optimization this fix can enable ?
>
> David
Xinliang David Li - Oct. 1, 2013, 9:58 p.m.
On Tue, Oct 1, 2013 at 2:37 PM, Xinliang David Li <davidxl@google.com> wrote:
> On Tue, Oct 1, 2013 at 10:31 AM, Cong Hou <congh@google.com> wrote:
>> The current uniform_vector_p() function only returns non-NULL when the
>> vector is directly a uniform vector. For example, for the following
>> gimple code:
>>
>> vect_cst_.15_91 = {_9, _9, _9, _9, _9, _9, _9, _9};
>>
>>
>> The current implementation can only detect that {_9, _9, _9, _9, _9,
>> _9, _9, _9} is a uniform vector, but fails to recognize
>> vect_cst_.15_91 is also one. This simple patch searches through
>> assignment chains to find more uniform vectors.
>>
>>
>> thanks,
>> Cong
>>
>>
>>
>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
>> index 45c1667..b42f8a9 100644
>> --- a/gcc/ChangeLog
>> +++ b/gcc/ChangeLog
>> @@ -1,3 +1,9 @@
>> +2013-10-01  Cong Hou  <congh@google.com>
>> +
>> +       * tree.c: Improve the function uniform_vector_p() so that a
>> +       vector assigned with a uniform vector is also treated as a
>> +       uniform vector.
>> +
>> diff --git a/gcc/tree.c b/gcc/tree.c
>> index 1c881e4..1d6d894 100644
>> --- a/gcc/tree.c
>> +++ b/gcc/tree.c
>> @@ -10297,6 +10297,17 @@ uniform_vector_p (const_tree vec)
>>        return first;
>>      }
>>
>> +  if (TREE_CODE (vec) == SSA_NAME)
>> +    {
>> +      gimple def = SSA_NAME_DEF_STMT (vec);
>> +      if (gimple_code (def) == GIMPLE_ASSIGN)
>
>
> do  this:
>
>              if (is_gimple_assign (def) && gimple_assign_copy_p (def))


Wrong comment from me. Should be

     if (gimple_assign_single_p (def))
      ..

David

>
>> +        {
>> +          tree rhs = gimple_op (def, 1);
>> +          if (VECTOR_TYPE_P (TREE_TYPE (rhs)))
>> +            return uniform_vector_p (rhs);
>> +        }
>> +    }
>> +
>>    return NULL_TREE;
>>  }
>
> Do you have a test case showing what missed optimization this fix can enable ?
>
> David
Richard Guenther - Oct. 2, 2013, 8:40 a.m.
On Tue, Oct 1, 2013 at 7:31 PM, Cong Hou <congh@google.com> wrote:
> The current uniform_vector_p() function only returns non-NULL when the
> vector is directly a uniform vector. For example, for the following
> gimple code:
>
> vect_cst_.15_91 = {_9, _9, _9, _9, _9, _9, _9, _9};
>
>
> The current implementation can only detect that {_9, _9, _9, _9, _9,
> _9, _9, _9} is a uniform vector, but fails to recognize
> vect_cst_.15_91 is also one. This simple patch searches through
> assignment chains to find more uniform vectors.
>

Changing uniform_vector_p looks wrong - it is a predicate on
GENERIC and you are adding SSA specifics to it.

I suggest you simply lookup the def of the SSA name for the
example you give in later mail.

Richard.

> thanks,
> Cong
>
>
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 45c1667..b42f8a9 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2013-10-01  Cong Hou  <congh@google.com>
> +
> +       * tree.c: Improve the function uniform_vector_p() so that a
> +       vector assigned with a uniform vector is also treated as a
> +       uniform vector.
> +
> diff --git a/gcc/tree.c b/gcc/tree.c
> index 1c881e4..1d6d894 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -10297,6 +10297,17 @@ uniform_vector_p (const_tree vec)
>        return first;
>      }
>
> +  if (TREE_CODE (vec) == SSA_NAME)
> +    {
> +      gimple def = SSA_NAME_DEF_STMT (vec);
> +      if (gimple_code (def) == GIMPLE_ASSIGN)
> +        {
> +          tree rhs = gimple_op (def, 1);
> +          if (VECTOR_TYPE_P (TREE_TYPE (rhs)))
> +            return uniform_vector_p (rhs);
> +        }
> +    }
> +
>    return NULL_TREE;
>  }

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 45c1667..b42f8a9 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@ 
+2013-10-01  Cong Hou  <congh@google.com>
+
+       * tree.c: Improve the function uniform_vector_p() so that a
+       vector assigned with a uniform vector is also treated as a
+       uniform vector.
+
diff --git a/gcc/tree.c b/gcc/tree.c
index 1c881e4..1d6d894 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -10297,6 +10297,17 @@  uniform_vector_p (const_tree vec)
       return first;
     }

+  if (TREE_CODE (vec) == SSA_NAME)
+    {
+      gimple def = SSA_NAME_DEF_STMT (vec);
+      if (gimple_code (def) == GIMPLE_ASSIGN)
+        {
+          tree rhs = gimple_op (def, 1);
+          if (VECTOR_TYPE_P (TREE_TYPE (rhs)))
+            return uniform_vector_p (rhs);
+        }
+    }
+
   return NULL_TREE;
 }