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] PING:fold two vector_cst in const_binop


On Tue, Apr 7, 2009 at 2:19 PM, Bingfeng Mei <bmei@broadcom.com> wrote:
> Hello, Richard,
>
> The following is the patch I submitted before, which was not appropriate for late stage of 4.4. I retested the patch on current trunk.
>
> --
> GCC fails to fold two vector_cst in ccp1 pass into one for vector modes that are natively supported by target. For example, the following test will fail in powerpc target with -maltivec turned on (v4si is supported by altivec natively). Ironically, if the vector mode is not supported natively by target, the folding can be successful through scalar operations :-)
>
> /* { dg-do compile } */
> /* { dg-options "-O2 -fdump-tree-ccp1" } */
>
> typedef unsigned int v4si __attribute__ ((vector_size (16)));
>
> v4si c;
>
> void foo()
> {
> ?v4si a = { 1, 2, 3, 4 };
> ?v4si b = { 5, 6, 7, 8 };
> ?c = a + b;
> }
>
> /* { dg-final { scan-tree-dump-times "c =.* { 6, 8, 10, 12 }" 1 "ccp1" } } */
> /* { dg-final { cleanup-tree-dump "ccp1" } } */
>
>
> GCC produces following intermediate code (.ccp1):
>
> ;; Function foo (foo)
>
> foo ()
> {
> ?v4si c.0;
>
> <bb 2>:
> ?c.0_3 = { 1, 2, 3, 4 } + { 5, 6, 7, 8 };
> ?c ={v} c.0_3;
> ?return;
>
> }
>
> The following patch fixes this problem. With the patch, GCC produces following intermediate code (.ccp1)
>
>
> ;; Function foo (foo)
>
> foo ()
> {
> <bb 2>:
> ?c ={v} { 6, 8, 10, 12 };
> ?return;
>
> }
>
> The patch was tested on x86_64-unknown-linux-gnu. I am not sure how to generalize above test case, since different targets support different vector modes. OK for mainline?
>
> Bingfeng Mei
> Broadcom UK
>
>
> 2009-04-07 Bingfeng Mei <bmei@broadcom.com>
>
> ? ? ? ?* fold-const.c (const_binop): Combine two VECTOR_CST under operation CODE to produce a new one
>
> Index: fold-const.c
> ===================================================================
> --- fold-const.c ? ? ? ?(revision 145659)
> +++ fold-const.c ? ? ? ?(working copy)
> @@ -1998,6 +1998,51 @@
> ? ? ? ?return build_complex (type, real, imag);
> ? ? }
>
> + ?if (TREE_CODE (arg1) == VECTOR_CST)

  && TREE_CODE (arg2) == VECTOR_CST)

as if we are ever called for shifts the shift argument may be a scalar.

> + ? ?{
> + ? ? ?tree type = TREE_TYPE(arg1);
> + ? ? ?int count = TYPE_VECTOR_SUBPARTS (type), i;
> + ? ? ?bool fail = false;
> + ? ? ?tree elements1, elements2, list = NULL_TREE;
> + ? ? ?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 = build_int_cst(TREE_TYPE(type), 0);

Hmm, where is this documented?   build_zero_vector fills in
explicit zero members.  Also using an integer type here is wrong
for floating point vectors.  Please instead bail out if this happens
on either vector.

> + ? ? ? ? ?else
> + ? ? ? ? ? ?{
> + ? ? ? ? ? ? ?elem1 = TREE_VALUE(elements1);
> + ? ? ? ? ? ? ?elements1 = TREE_CHAIN (elements1);
> + ? ? ? ? ? ?}
> +
> + ? ? ? ? ?if(!elements2)
> + ? ? ? ? ? ?elem2 = build_int_cst(TREE_TYPE(type), 0);
> + ? ? ? ? ?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)
> + ? ? ? ? ? ?{
> + ? ? ? ? ? ? ?fail = true;
> + ? ? ? ? ? ? ?break;

Just return NULL_TREE directly in this case,

> + ? ? ? ? ? ?}
> +
> + ? ? ? ? ?list = tree_cons (NULL_TREE, elem, list);
> + ? ? ? }
> + ? ? ?if(!fail)

no need for the fail variable.

> + ? ? ? ?return build_vector(type, nreverse(list));

Instead of nreversing here build the element list in the correct
order.

Please re-sumbit with the suggested changes and updated
testing.

Thanks,
Richard.

> + ? ?}
> ? return NULL_TREE;
> ?}
>
>


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