Re: Constant folding of VEC_COND_EXPR

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

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
>