Re: [PATCH] PING:fold two vector_cst in const_binop

[Richard Guenther <richard dot guenther at gmail dot com>]

On Wed, Apr 8, 2009 at 2:01 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> Richard,
> Here is the modified version. It works both on integer and floating-point. I couldn't avoid nreverse by reverse counting because number of elements are not the same as vector size. Basically, my code is similar to following code in the fold_unary function:
>
> ? ? ?else if (TREE_CODE (arg0) == VECTOR_CST)
> ? ? ? ?{
> ? ? ? ? ?tree elements = TREE_VECTOR_CST_ELTS (arg0), elem, list = NULL_TREE;
> ? ? ? ? ?int count = TYPE_VECTOR_SUBPARTS (type), i;
>
> ? ? ? ? ?for (i = 0; i < count; i++)
> ? ? ? ? ? ?{
> ? ? ? ? ? ? ?if (elements)
> ? ? ? ? ? ? ? ?{
> ? ? ? ? ? ? ? ? ?elem = TREE_VALUE (elements);
> ? ? ? ? ? ? ? ? ?elem = fold_unary (BIT_NOT_EXPR, TREE_TYPE (type), elem);
> ? ? ? ? ? ? ? ? ?if (elem == NULL_TREE)
> ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ? ? ? ?elements = TREE_CHAIN (elements);
> ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ?else
> ? ? ? ? ? ? ? ?elem = build_int_cst (TREE_TYPE (type), -1);
> ? ? ? ? ? ? ?list = tree_cons (NULL_TREE, elem, list);
> ? ? ? ? ? ?}
> ? ? ? ? ?if (i == count)
> ? ? ? ? ? ?return build_vector (type, nreverse (list));
> ? ? ? ?}
>
>
> Another thing I am not sure is how to form the test case because which vectors used here are target-specific. Maybe I can put the test in gcc.dg/vect directory and make use of vect.exp infrastructure.
>
> Thanks,
> Bingfeng
>
>
> test code:
>
> /* { dg-do compile } */
> /* { dg-options "-O2 -fdump-tree-ccp1" } */
>
> typedef unsigned int v4si __attribute__ ((vector_size (16)));
> typedef float v4sf __attribute__ ((vector_size (16)));
>
> v4si c;
> void foo1()
> {
> ?v4si a = { 1, 2, 3, 4 };
> ?v4si b = { 5, 6, 7, 8 };
> ?c = a + b;
> }
>
> void foo2()
> {
> ?v4si a = { 1, 2};
> ?v4si b = { 5, 6, 7 };
> ?c = a + b;
> }
>
> v4sf d;
> void foo3()
> {
> ?v4sf a = { 1.0, 2.0, 3.0, 4.0};
> ?v4sf b = { 5.0, 6.0, 7.0, 8.0};
> ?d = a + b;
> }
>
> /* { dg-final { scan-tree-dump-times "c =.* { 6, 8, 10, 12 }" 1 "ccp1" } } */
> /* { dg-final { scan-tree-dump-times "c =.* { 6, 8, 7, 0 }" 1 "ccp1" } } */
> /* { dg-final { scan-tree-dump-times "c =.* { 6.0e+0, 8.0e+0, 1.0e+1, 1.2e+1 }" 1 "ccp1" } } */
> /* { dg-final { cleanup-tree-dump "ccp1" } } */
>
> ccp1 file:
>
> ;; Function foo1 (foo1)
>
> foo1 ()
> {
> <bb 2>:
> ?c = { 6, 8, 10, 12 };
> ?return;
>
> }
>
>
>
> ;; Function foo2 (foo2)
>
> foo2 ()
> {
> <bb 2>:
> ?c = { 6, 8, 7, 0 };
> ?return;
>
> }
>
>
>
> ;; Function foo3 (foo3)
>
> foo3 ()
> {
> <bb 2>:
> ?d = { 6.0e+0, 8.0e+0, 1.0e+1, 1.2e+1 };
> ?return;
>
> }
>
>
> The updated patch:
> Index: fold-const.c
> ===================================================================
> --- fold-const.c ? ? ? ?(revision 145659)
> +++ fold-const.c ? ? ? ?(working copy)
> @@ -141,6 +141,7 @@
> ?static tree fold_negate_const (tree, tree);
> ?static tree fold_not_const (tree, tree);
> ?static tree fold_relational_const (enum tree_code, tree, tree, tree);
> +static tree fold_convert_const (enum tree_code, tree, tree);
>
>
> ?/* We know that A1 + B1 = SUM1, using 2's complement arithmetic and ignoring
> @@ -1998,6 +1999,50 @@
> ? ? ? ?return build_complex (type, real, imag);
> ? ? }
>
> + ?if (TREE_CODE (arg1) == VECTOR_CST)

&& TREE_CODE (arg2) == VECTOR_CST)

> + ? ?{
> + ? ? ?tree type = TREE_TYPE(arg1);
> + ? ? ?int count = TYPE_VECTOR_SUBPARTS (type), i;
> + ? ? ?tree elements1, elements2, list = NULL_TREE;
> +
> + ? ? ?if(TREE_CODE(arg2) != VECTOR_CST)
> + ? ? ? ?return NULL_TREE;

and omit the check here.

> + ? ? ?elements1 = TREE_VECTOR_CST_ELTS (arg1);
> + ? ? ?elements2 = TREE_VECTOR_CST_ELTS (arg2);
> +
> + ? ? ?for (i = 0; i < count; i++)
> + ? ? ? {
> + ? ? ? ? ?tree elem1, elem2, elem;
> +
> + ? ? ? ? ?/* The trailing elements can be empty and should be treated as 0 */
> + ? ? ? ? ?if(!elements1)
> + ? ? ? ? ? ?elem1 = fold_convert_const (NOP_EXPR, TREE_TYPE (type), integer_zero_node);

just use fold_convert (TREE_TYPE (type), integer_zero_node) so
you do not need the fold_convert_const declaration.

> + ? ? ? ? ?else
> + ? ? ? ? ? ?{
> + ? ? ? ? ? ? ?elem1 = TREE_VALUE(elements1);
> + ? ? ? ? ? ? ?elements1 = TREE_CHAIN (elements1);
> + ? ? ? ? ? ?}
> +
> + ? ? ? ? ?if(!elements2)
> + ? ? ? ? ? ?elem2 = fold_convert_const (NOP_EXPR, TREE_TYPE (type), integer_zero_node);

Likewise.

> + ? ? ? ? ?else
> + ? ? ? ? ? ?{
> + ? ? ? ? ? ? ?elem2 = TREE_VALUE(elements2);
> + ? ? ? ? ? ? ?elements2 = TREE_CHAIN (elements2);
> + ? ? ? ? ? ?}
> +
> + ? ? ? ? ?elem = const_binop (code, elem1, elem2, notrunc);
> +
> + ? ? ? ? ?/* It is possible that const_binop cannot handle the given
> + ? ? ? ? ? ?code and return NULL_TREE */
> + ? ? ? ? ?if(elem == NULL_TREE)
> + ? ? ? ? ? ?return NULL_TREE;
> +
> + ? ? ? ? ?list = tree_cons (NULL_TREE, elem, list);
> + ? ? ? }
> + ? ? ?return build_vector(type, nreverse(list));
> + ? ?}
> ? return NULL_TREE;
> ?}

The patch is ok with this change with a proper ChangeLog entry
and if it passes bootstrap & regtest.

Thanks,
Richard.