diff mbox

Constant folding of VEC_COND_EXPR

Message ID alpine.DEB.2.02.1303311510370.25614@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse March 31, 2013, 1:31 p.m. UTC
Hello,

this adds constant folding of VEC_COND_EXPR at the tree level by 
forwarding to the VEC_PERM_EXPR code (a merge is a special case of a 
permutation). The CONSTRUCTOR case may be unreachable for now (it will 
probably need an extra piece of code in tree-ssa-forwprop.c), but it seems 
better to add it at the same time.

bootstrap+testsuite on x86_64-linux-gnu.

2013-03-31  Marc Glisse  <marc.glisse@inria.fr>

 	PR tree-optimization/56790
 	* fold-const.c (fold_ternary_loc) <VEC_COND_EXPR>: Add constant folding.

testsuite/
 	* g++.dg/ext/pr56790-1.C: New testcase.

Comments

Andrew Pinski March 31, 2013, 4:59 p.m. UTC | #1
On Sun, Mar 31, 2013 at 6:31 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> this adds constant folding of VEC_COND_EXPR at the tree level by forwarding
> to the VEC_PERM_EXPR code (a merge is a special case of a permutation). The
> CONSTRUCTOR case may be unreachable for now (it will probably need an extra
> piece of code in tree-ssa-forwprop.c), but it seems better to add it at the
> same time.
>
> bootstrap+testsuite on x86_64-linux-gnu.
>
> 2013-03-31  Marc Glisse  <marc.glisse@inria.fr>
>
>         PR tree-optimization/56790
>         * fold-const.c (fold_ternary_loc) <VEC_COND_EXPR>: Add constant
> folding.
>
> testsuite/
>         * g++.dg/ext/pr56790-1.C: New testcase.
>
> --
> Marc Glisse
> Index: gcc/testsuite/g++.dg/ext/pr56790-1.C
> ===================================================================
> --- gcc/testsuite/g++.dg/ext/pr56790-1.C        (revision 0)
> +++ gcc/testsuite/g++.dg/ext/pr56790-1.C        (revision 0)
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-ccp1" } */
> +
> +typedef long vec __attribute__ ((vector_size (2 * sizeof (long))));
> +
> +vec f (void)
> +{
> +  vec a = {  5,  7 };
> +  vec b = { 11, 13 };
> +  vec m = { -1,  0 };
> +  return m ? a : b;
> +}
> +
> +/* { dg-final { scan-tree-dump "{ 5, 13 }" "ccp1" } } */
> +/* { dg-final { scan-tree-dump-not "VEC_COND_EXPR" "ccp1" } } */
> +/* { dg-final { cleanup-tree-dump "ccp1" } } */
>
> Property changes on: gcc/testsuite/g++.dg/ext/pr56790-1.C
> ___________________________________________________________________
> Added: svn:keywords
>    + Author Date Id Revision URL
> Added: svn:eol-style
>    + native
>
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c    (revision 197284)
> +++ gcc/fold-const.c    (working copy)
> @@ -13917,20 +13917,43 @@ fold_ternary_loc (location_t loc, enum t
>                    || VOID_TYPE_P (type)))
>             return pedantic_non_lvalue_loc (loc, tem);
>           return NULL_TREE;
>         }
>        else if (TREE_CODE (arg0) == VECTOR_CST)
>         {
>           if (integer_all_onesp (arg0))
>             return pedantic_omit_one_operand_loc (loc, type, arg1, arg2);
>           if (integer_zerop (arg0))
>             return pedantic_omit_one_operand_loc (loc, type, arg2, arg1);
> +
> +         if ((TREE_CODE (arg1) == VECTOR_CST
> +              || TREE_CODE (arg1) == CONSTRUCTOR)
> +             && (TREE_CODE (arg2) == VECTOR_CST
> +                 || TREE_CODE (arg2) == CONSTRUCTOR))
> +           {
> +             unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i;
> +             unsigned char *sel = XALLOCAVEC (unsigned char, nelts);
> +             gcc_assert (nelts == VECTOR_CST_NELTS (arg0));
> +             for (i = 0; i < nelts; i++)
> +               {
> +                 tree val = VECTOR_CST_ELT (arg0, i);
> +                 if (integer_all_onesp (val))
> +                   sel[i] = i;
> +                 else if (integer_zerop (val))
> +                   sel[i] = nelts + i;
> +                 else
> +                   gcc_unreachable ();

I think this gcc_unreachable here is incorrect as it could cause an
internal compiler error for "target dependent code"
Try for:
typedef long vec __attribute__ ((vector_size (2 * sizeof (long))));

vec f (void)
{
  vec a = {  5,  7 };
  vec b = { 11, 13 };
  vec m = { 3,  2 };
  return m ? a : b;
}

I think for the above case we don't want to do any constant folding.

Thanks,
Andrew Pinski

> +               }
> +             tree t = fold_vec_perm (type, arg1, arg2, sel);
> +             if (t != NULL_TREE)
> +               return t;
> +           }
>         }
>
>        if (operand_equal_p (arg1, op2, 0))
>         return pedantic_omit_one_operand_loc (loc, type, arg1, arg0);
>
>        /* If we have A op B ? A : C, we may be able to convert this to a
>          simpler expression, depending on the operation and the values
>          of B and C.  Signed zeros prevent all of these transformations,
>          for reasons given above each one.
>
>
Marc Glisse March 31, 2013, 5:20 p.m. UTC | #2
On Sun, 31 Mar 2013, Andrew Pinski wrote:

> On Sun, Mar 31, 2013 at 6:31 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> this adds constant folding of VEC_COND_EXPR at the tree level by forwarding
>> to the VEC_PERM_EXPR code (a merge is a special case of a permutation). The
>> CONSTRUCTOR case may be unreachable for now (it will probably need an extra
>> piece of code in tree-ssa-forwprop.c), but it seems better to add it at the
>> same time.
>>
>> bootstrap+testsuite on x86_64-linux-gnu.
>>
>> 2013-03-31  Marc Glisse  <marc.glisse@inria.fr>
>>
>>         PR tree-optimization/56790
>>         * fold-const.c (fold_ternary_loc) <VEC_COND_EXPR>: Add constant
>> folding.
>>
>> testsuite/
>>         * g++.dg/ext/pr56790-1.C: New testcase.
>>
>> --
>> Marc Glisse
>> Index: gcc/testsuite/g++.dg/ext/pr56790-1.C
>> ===================================================================
>> --- gcc/testsuite/g++.dg/ext/pr56790-1.C        (revision 0)
>> +++ gcc/testsuite/g++.dg/ext/pr56790-1.C        (revision 0)
>> @@ -0,0 +1,16 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fdump-tree-ccp1" } */
>> +
>> +typedef long vec __attribute__ ((vector_size (2 * sizeof (long))));
>> +
>> +vec f (void)
>> +{
>> +  vec a = {  5,  7 };
>> +  vec b = { 11, 13 };
>> +  vec m = { -1,  0 };
>> +  return m ? a : b;
>> +}
>> +
>> +/* { dg-final { scan-tree-dump "{ 5, 13 }" "ccp1" } } */
>> +/* { dg-final { scan-tree-dump-not "VEC_COND_EXPR" "ccp1" } } */
>> +/* { dg-final { cleanup-tree-dump "ccp1" } } */
>>
>> Property changes on: gcc/testsuite/g++.dg/ext/pr56790-1.C
>> ___________________________________________________________________
>> Added: svn:keywords
>>    + Author Date Id Revision URL
>> Added: svn:eol-style
>>    + native
>>
>> Index: gcc/fold-const.c
>> ===================================================================
>> --- gcc/fold-const.c    (revision 197284)
>> +++ gcc/fold-const.c    (working copy)
>> @@ -13917,20 +13917,43 @@ fold_ternary_loc (location_t loc, enum t
>>                    || VOID_TYPE_P (type)))
>>             return pedantic_non_lvalue_loc (loc, tem);
>>           return NULL_TREE;
>>         }
>>        else if (TREE_CODE (arg0) == VECTOR_CST)
>>         {
>>           if (integer_all_onesp (arg0))
>>             return pedantic_omit_one_operand_loc (loc, type, arg1, arg2);
>>           if (integer_zerop (arg0))
>>             return pedantic_omit_one_operand_loc (loc, type, arg2, arg1);
>> +
>> +         if ((TREE_CODE (arg1) == VECTOR_CST
>> +              || TREE_CODE (arg1) == CONSTRUCTOR)
>> +             && (TREE_CODE (arg2) == VECTOR_CST
>> +                 || TREE_CODE (arg2) == CONSTRUCTOR))
>> +           {
>> +             unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i;
>> +             unsigned char *sel = XALLOCAVEC (unsigned char, nelts);
>> +             gcc_assert (nelts == VECTOR_CST_NELTS (arg0));
>> +             for (i = 0; i < nelts; i++)
>> +               {
>> +                 tree val = VECTOR_CST_ELT (arg0, i);
>> +                 if (integer_all_onesp (val))
>> +                   sel[i] = i;
>> +                 else if (integer_zerop (val))
>> +                   sel[i] = nelts + i;
>> +                 else
>> +                   gcc_unreachable ();
>
> I think this gcc_unreachable here is incorrect as it could cause an
> internal compiler error for "target dependent code"
> Try for:
> typedef long vec __attribute__ ((vector_size (2 * sizeof (long))));
>
> vec f (void)
> {
>  vec a = {  5,  7 };
>  vec b = { 11, 13 };
>  vec m = { 3,  2 };
>  return m ? a : b;
> }
>
> I think for the above case we don't want to do any constant folding.

For vectors, we decided in 4.8 that x ? y : z would mean vec_cond_expr <x 
!= 0, y, z>, and that is what the C++ front-end generates, so your 
testcase works fine and returns a.

Re-reading doc/generic.texi, I see:

"If an element of the first operand evaluates to a zero value, the 
corresponding element of the result is taken from the third operand. If it 
evaluates to a minus one value, it is taken from the second operand. It 
should never evaluate to any other value currently, but optimizations 
should not rely on that property."

Well, at least I am not silently relying on that property, but it looks 
like you are right and I am supposed to leave those (impossible) values 
alone.
Richard Biener April 2, 2013, 9:33 a.m. UTC | #3
On Sun, Mar 31, 2013 at 7:20 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Sun, 31 Mar 2013, Andrew Pinski wrote:
>
>> On Sun, Mar 31, 2013 at 6:31 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>
>>> Hello,
>>>
>>> this adds constant folding of VEC_COND_EXPR at the tree level by
>>> forwarding
>>> to the VEC_PERM_EXPR code (a merge is a special case of a permutation).
>>> The
>>> CONSTRUCTOR case may be unreachable for now (it will probably need an
>>> extra
>>> piece of code in tree-ssa-forwprop.c), but it seems better to add it at
>>> the
>>> same time.
>>>
>>> bootstrap+testsuite on x86_64-linux-gnu.
>>>
>>> 2013-03-31  Marc Glisse  <marc.glisse@inria.fr>
>>>
>>>         PR tree-optimization/56790
>>>         * fold-const.c (fold_ternary_loc) <VEC_COND_EXPR>: Add constant
>>> folding.
>>>
>>> testsuite/
>>>         * g++.dg/ext/pr56790-1.C: New testcase.
>>>
>>> --
>>> Marc Glisse
>>> Index: gcc/testsuite/g++.dg/ext/pr56790-1.C
>>> ===================================================================
>>> --- gcc/testsuite/g++.dg/ext/pr56790-1.C        (revision 0)
>>> +++ gcc/testsuite/g++.dg/ext/pr56790-1.C        (revision 0)
>>> @@ -0,0 +1,16 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O2 -fdump-tree-ccp1" } */
>>> +
>>> +typedef long vec __attribute__ ((vector_size (2 * sizeof (long))));
>>> +
>>> +vec f (void)
>>> +{
>>> +  vec a = {  5,  7 };
>>> +  vec b = { 11, 13 };
>>> +  vec m = { -1,  0 };
>>> +  return m ? a : b;
>>> +}
>>> +
>>> +/* { dg-final { scan-tree-dump "{ 5, 13 }" "ccp1" } } */
>>> +/* { dg-final { scan-tree-dump-not "VEC_COND_EXPR" "ccp1" } } */
>>> +/* { dg-final { cleanup-tree-dump "ccp1" } } */
>>>
>>> Property changes on: gcc/testsuite/g++.dg/ext/pr56790-1.C
>>> ___________________________________________________________________
>>> Added: svn:keywords
>>>    + Author Date Id Revision URL
>>> Added: svn:eol-style
>>>    + native
>>>
>>> Index: gcc/fold-const.c
>>> ===================================================================
>>> --- gcc/fold-const.c    (revision 197284)
>>> +++ gcc/fold-const.c    (working copy)
>>> @@ -13917,20 +13917,43 @@ fold_ternary_loc (location_t loc, enum t
>>>                    || VOID_TYPE_P (type)))
>>>             return pedantic_non_lvalue_loc (loc, tem);
>>>           return NULL_TREE;
>>>         }
>>>        else if (TREE_CODE (arg0) == VECTOR_CST)
>>>         {
>>>           if (integer_all_onesp (arg0))
>>>             return pedantic_omit_one_operand_loc (loc, type, arg1, arg2);
>>>           if (integer_zerop (arg0))
>>>             return pedantic_omit_one_operand_loc (loc, type, arg2, arg1);
>>> +
>>> +         if ((TREE_CODE (arg1) == VECTOR_CST
>>> +              || TREE_CODE (arg1) == CONSTRUCTOR)
>>> +             && (TREE_CODE (arg2) == VECTOR_CST
>>> +                 || TREE_CODE (arg2) == CONSTRUCTOR))
>>> +           {
>>> +             unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i;
>>> +             unsigned char *sel = XALLOCAVEC (unsigned char, nelts);
>>> +             gcc_assert (nelts == VECTOR_CST_NELTS (arg0));
>>> +             for (i = 0; i < nelts; i++)
>>> +               {
>>> +                 tree val = VECTOR_CST_ELT (arg0, i);
>>> +                 if (integer_all_onesp (val))
>>> +                   sel[i] = i;
>>> +                 else if (integer_zerop (val))
>>> +                   sel[i] = nelts + i;
>>> +                 else
>>> +                   gcc_unreachable ();
>>
>>
>> I think this gcc_unreachable here is incorrect as it could cause an
>> internal compiler error for "target dependent code"
>> Try for:
>> typedef long vec __attribute__ ((vector_size (2 * sizeof (long))));
>>
>> vec f (void)
>> {
>>  vec a = {  5,  7 };
>>  vec b = { 11, 13 };
>>  vec m = { 3,  2 };
>>  return m ? a : b;
>> }
>>
>> I think for the above case we don't want to do any constant folding.
>
>
> For vectors, we decided in 4.8 that x ? y : z would mean vec_cond_expr <x !=
> 0, y, z>, and that is what the C++ front-end generates, so your testcase
> works fine and returns a.
>
> Re-reading doc/generic.texi, I see:
>
> "If an element of the first operand evaluates to a zero value, the
> corresponding element of the result is taken from the third operand. If it
> evaluates to a minus one value, it is taken from the second operand. It
> should never evaluate to any other value currently, but optimizations should
> not rely on that property."
>
> Well, at least I am not silently relying on that property, but it looks like
> you are right and I am supposed to leave those (impossible) values alone.

Yes, in general just don't fold if you hit unexpected things (return NULL_TREE).
Ok with that change.

Thanks,
Richard.

> --
> Marc Glisse
>
diff mbox

Patch

Index: gcc/testsuite/g++.dg/ext/pr56790-1.C
===================================================================
--- gcc/testsuite/g++.dg/ext/pr56790-1.C	(revision 0)
+++ gcc/testsuite/g++.dg/ext/pr56790-1.C	(revision 0)
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-ccp1" } */
+
+typedef long vec __attribute__ ((vector_size (2 * sizeof (long))));
+
+vec f (void)
+{
+  vec a = {  5,  7 };
+  vec b = { 11, 13 };
+  vec m = { -1,  0 };
+  return m ? a : b;
+}
+
+/* { dg-final { scan-tree-dump "{ 5, 13 }" "ccp1" } } */
+/* { dg-final { scan-tree-dump-not "VEC_COND_EXPR" "ccp1" } } */
+/* { dg-final { cleanup-tree-dump "ccp1" } } */

Property changes on: gcc/testsuite/g++.dg/ext/pr56790-1.C
___________________________________________________________________
Added: svn:keywords
   + Author Date Id Revision URL
Added: svn:eol-style
   + native

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 197284)
+++ gcc/fold-const.c	(working copy)
@@ -13917,20 +13917,43 @@  fold_ternary_loc (location_t loc, enum t
                   || VOID_TYPE_P (type)))
 	    return pedantic_non_lvalue_loc (loc, tem);
 	  return NULL_TREE;
 	}
       else if (TREE_CODE (arg0) == VECTOR_CST)
 	{
 	  if (integer_all_onesp (arg0))
 	    return pedantic_omit_one_operand_loc (loc, type, arg1, arg2);
 	  if (integer_zerop (arg0))
 	    return pedantic_omit_one_operand_loc (loc, type, arg2, arg1);
+
+	  if ((TREE_CODE (arg1) == VECTOR_CST
+	       || TREE_CODE (arg1) == CONSTRUCTOR)
+	      && (TREE_CODE (arg2) == VECTOR_CST
+		  || TREE_CODE (arg2) == CONSTRUCTOR))
+	    {
+	      unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i;
+	      unsigned char *sel = XALLOCAVEC (unsigned char, nelts);
+	      gcc_assert (nelts == VECTOR_CST_NELTS (arg0));
+	      for (i = 0; i < nelts; i++)
+		{
+		  tree val = VECTOR_CST_ELT (arg0, i);
+		  if (integer_all_onesp (val))
+		    sel[i] = i;
+		  else if (integer_zerop (val))
+		    sel[i] = nelts + i;
+		  else
+		    gcc_unreachable ();
+		}
+	      tree t = fold_vec_perm (type, arg1, arg2, sel);
+	      if (t != NULL_TREE)
+		return t;
+	    }
 	}
 
       if (operand_equal_p (arg1, op2, 0))
 	return pedantic_omit_one_operand_loc (loc, type, arg1, arg0);
 
       /* If we have A op B ? A : C, we may be able to convert this to a
 	 simpler expression, depending on the operation and the values
 	 of B and C.  Signed zeros prevent all of these transformations,
 	 for reasons given above each one.