Re: integer_each_onep + vector bug

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

On Fri, Sep 12, 2014 at 12:04 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> here is a new predicate which tests if a number is 1, or (for vector and
> complex) a collection of 1. The only difference with integer_onep should be
> for complex where it wants 1+i and not 1. The main use would be in the match
> branch, so I didn't waste too much time adding many uses, I just added a
> couple (so the function won't be garbage collected in some refactoring)
> though I wasn't able to create a testcase (for complex int, '&' or '^' are
> rejected and '~' means conjugate).
>
> While looking for potential uses for integer_each_onep, I couldn't help
> noticing a few wrong optimizations for vectors, that I am fixing at the same
> time.

Yeah, the transforms you disable are questionable for GIMPLE anyway
as (X & 1) == 0 needs to have boolean type there and we like the
other forms more.  This looks like a transform targeted at RTL expansion
to me (or one that is profitable if the expression feeds a conditional
which we can merge it into - like the (~X & Y) -> X < Y transforms
tree-ssa-forwprop.c does.

Ok.

Thanks,
Richard.

>
> Bootstrap+testsuite on x86_64-linux-gnu.
>
>
> 2014-09-12  Marc Glisse  <marc.glisse@inria.fr>
>
> gcc/
>         * tree.c (integer_each_onep): New function.
>         * tree.h (integer_each_onep): Declare it.
>         * fold-const.c (fold_binary_loc): Use it for ~A + 1 to -A and
>         -A - 1 to ~A.  Disable (X & 1) ^ 1, (X ^ 1) & 1 and ~X & 1 to
>         (X & 1) == 0 for vector and complex.
> gcc/testsuite/
>         * gcc.dg/vec-andxor1.c: New file.
>
>
> --
> Marc Glisse
> Index: fold-const.c
> ===================================================================
> --- fold-const.c        (revision 215179)
> +++ fold-const.c        (working copy)
> @@ -10085,21 +10085,21 @@ fold_binary_loc (location_t loc,
>           && (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)
>         return fold_build2_loc (loc, MINUS_EXPR, type,
>                             fold_convert_loc (loc, type, arg1),
>                             fold_convert_loc (loc, type,
>                                               TREE_OPERAND (arg0, 0)));
>
>        if (INTEGRAL_TYPE_P (type) || VECTOR_INTEGER_TYPE_P (type))
>         {
>           /* Convert ~A + 1 to -A.  */
>           if (TREE_CODE (arg0) == BIT_NOT_EXPR
> -             && integer_onep (arg1))
> +             && integer_each_onep (arg1))
>             return fold_build1_loc (loc, NEGATE_EXPR, type,
>                                 fold_convert_loc (loc, type,
>                                                   TREE_OPERAND (arg0, 0)));
>
>           /* ~X + X is -1.  */
>           if (TREE_CODE (arg0) == BIT_NOT_EXPR
>               && !TYPE_OVERFLOW_TRAPS (type))
>             {
>               tree tem = TREE_OPERAND (arg0, 0);
>
> @@ -10612,23 +10612,22 @@ fold_binary_loc (location_t loc,
>        /* (-A) - B -> (-B) - A  where B is easily negated and we can swap.
> */
>        if (TREE_CODE (arg0) == NEGATE_EXPR
>           && negate_expr_p (arg1)
>           && reorder_operands_p (arg0, arg1))
>         return fold_build2_loc (loc, MINUS_EXPR, type,
>                             fold_convert_loc (loc, type,
>                                               negate_expr (arg1)),
>                             fold_convert_loc (loc, type,
>                                               TREE_OPERAND (arg0, 0)));
>        /* Convert -A - 1 to ~A.  */
> -      if (TREE_CODE (type) != COMPLEX_TYPE
> -         && TREE_CODE (arg0) == NEGATE_EXPR
> -         && integer_onep (arg1)
> +      if (TREE_CODE (arg0) == NEGATE_EXPR
> +         && integer_each_onep (arg1)
>           && !TYPE_OVERFLOW_TRAPS (type))
>         return fold_build1_loc (loc, BIT_NOT_EXPR, type,
>                             fold_convert_loc (loc, type,
>                                               TREE_OPERAND (arg0, 0)));
>
>        /* Convert -1 - A to ~A.  */
>        if (TREE_CODE (type) != COMPLEX_TYPE
>           && integer_all_onesp (arg0))
>         return fold_build1_loc (loc, BIT_NOT_EXPR, type, op1);
>
> @@ -11377,20 +11376,21 @@ fold_binary_loc (location_t loc,
>        /* Convert ~X ^ C to X ^ ~C.  */
>        if (TREE_CODE (arg0) == BIT_NOT_EXPR
>           && TREE_CODE (arg1) == INTEGER_CST)
>         return fold_build2_loc (loc, code, type,
>                             fold_convert_loc (loc, type,
>                                               TREE_OPERAND (arg0, 0)),
>                             fold_build1_loc (loc, BIT_NOT_EXPR, type,
> arg1));
>
>        /* Fold (X & 1) ^ 1 as (X & 1) == 0.  */
>        if (TREE_CODE (arg0) == BIT_AND_EXPR
> +         && INTEGRAL_TYPE_P (type)
>           && integer_onep (TREE_OPERAND (arg0, 1))
>           && integer_onep (arg1))
>         return fold_build2_loc (loc, EQ_EXPR, type, arg0,
>                                 build_zero_cst (TREE_TYPE (arg0)));
>
>        /* Fold (X & Y) ^ Y as ~X & Y.  */
>        if (TREE_CODE (arg0) == BIT_AND_EXPR
>           && operand_equal_p (TREE_OPERAND (arg0, 1), arg1, 0))
>         {
>           tem = fold_convert_loc (loc, type, TREE_OPERAND (arg0, 0));
> @@ -11487,33 +11487,35 @@ fold_binary_loc (location_t loc,
>           && reorder_operands_p (arg0, TREE_OPERAND (arg1, 1)))
>         return omit_one_operand_loc (loc, type, arg0, TREE_OPERAND (arg1,
> 1));
>        /* X & (Y | X) is (Y, X).  */
>        if (TREE_CODE (arg1) == BIT_IOR_EXPR
>           && operand_equal_p (arg0, TREE_OPERAND (arg1, 1), 0)
>           && reorder_operands_p (arg0, TREE_OPERAND (arg1, 0)))
>         return omit_one_operand_loc (loc, type, arg0, TREE_OPERAND (arg1,
> 0));
>
>        /* Fold (X ^ 1) & 1 as (X & 1) == 0.  */
>        if (TREE_CODE (arg0) == BIT_XOR_EXPR
> +         && INTEGRAL_TYPE_P (type)
>           && integer_onep (TREE_OPERAND (arg0, 1))
>           && integer_onep (arg1))
>         {
>           tree tem2;
>           tem = TREE_OPERAND (arg0, 0);
>           tem2 = fold_convert_loc (loc, TREE_TYPE (tem), arg1);
>           tem2 = fold_build2_loc (loc, BIT_AND_EXPR, TREE_TYPE (tem),
>                                   tem, tem2);
>           return fold_build2_loc (loc, EQ_EXPR, type, tem2,
>                                   build_zero_cst (TREE_TYPE (tem)));
>         }
>        /* Fold ~X & 1 as (X & 1) == 0.  */
>        if (TREE_CODE (arg0) == BIT_NOT_EXPR
> +         && INTEGRAL_TYPE_P (type)
>           && integer_onep (arg1))
>         {
>           tree tem2;
>           tem = TREE_OPERAND (arg0, 0);
>           tem2 = fold_convert_loc (loc, TREE_TYPE (tem), arg1);
>           tem2 = fold_build2_loc (loc, BIT_AND_EXPR, TREE_TYPE (tem),
>                                   tem, tem2);
>           return fold_build2_loc (loc, EQ_EXPR, type, tem2,
>                                   build_zero_cst (TREE_TYPE (tem)));
>         }
> Index: testsuite/gcc.dg/vec-andxor1.c
> ===================================================================
> --- testsuite/gcc.dg/vec-andxor1.c      (revision 0)
> +++ testsuite/gcc.dg/vec-andxor1.c      (working copy)
> @@ -0,0 +1,17 @@
> +/* { dg-do run } */
> +/* { dg-options "-O" } */
> +
> +typedef int vec __attribute__((vector_size(4*sizeof(int))));
> +
> +__attribute__((noinline,noclone))
> +void f (vec *x) {
> +  *x = (*x & 1) ^ 1;
> +}
> +
> +int main() {
> +  vec x = { 1, 2, 3, 4 };
> +  f(&x);
> +  if (x[0] != 0 || x[1] != 1)
> +    __builtin_abort();
> +  return 0;
> +}
> Index: tree.c
> ===================================================================
> --- tree.c      (revision 215179)
> +++ tree.c      (working copy)
> @@ -2162,20 +2162,35 @@ integer_onep (const_tree expr)
>         for (i = 0; i < VECTOR_CST_NELTS (expr); ++i)
>           if (!integer_onep (VECTOR_CST_ELT (expr, i)))
>             return false;
>         return true;
>        }
>      default:
>        return false;
>      }
>  }
>
> +/* Return 1 if EXPR is the integer constant one.  For complex and vector,
> +   return 1 if every piece is the integer constant one.  */
> +
> +int
> +integer_each_onep (const_tree expr)
> +{
> +  STRIP_NOPS (expr);
> +
> +  if (TREE_CODE (expr) == COMPLEX_CST)
> +    return (integer_onep (TREE_REALPART (expr))
> +           && integer_onep (TREE_IMAGPART (expr)));
> +  else
> +    return integer_onep (expr);
> +}
> +
>  /* Return 1 if EXPR is an integer containing all 1's in as much precision
> as
>     it contains, or a complex or vector whose subparts are such integers.
> */
>
>  int
>  integer_all_onesp (const_tree expr)
>  {
>    STRIP_NOPS (expr);
>
>    if (TREE_CODE (expr) == COMPLEX_CST
>        && integer_all_onesp (TREE_REALPART (expr))
> Index: tree.h
> ===================================================================
> --- tree.h      (revision 215179)
> +++ tree.h      (working copy)
> @@ -3936,20 +3936,25 @@ extern tree uniform_vector_p (const_tree
>  extern vec<tree, va_gc> *ctor_to_vec (tree);
>
>  /* integer_zerop (tree x) is nonzero if X is an integer constant of value
> 0.  */
>
>  extern int integer_zerop (const_tree);
>
>  /* integer_onep (tree x) is nonzero if X is an integer constant of value 1.
> */
>
>  extern int integer_onep (const_tree);
>
> +/* integer_onep (tree x) is nonzero if X is an integer constant of value 1,
> or
> +   a vector or complex where each part is 1.  */
> +
> +extern int integer_each_onep (const_tree);
> +
>  /* integer_all_onesp (tree x) is nonzero if X is an integer constant
>     all of whose significant bits are 1.  */
>
>  extern int integer_all_onesp (const_tree);
>
>  /* integer_minus_onep (tree x) is nonzero if X is an integer constant of
>     value -1.  */
>
>  extern int integer_minus_onep (const_tree);
>
>