diff mbox series

apply unary op to both sides of (vec_cond x cst1 cst2)

Message ID alpine.DEB.2.02.1905191816330.13168@grove.saclay.inria.fr
State New
Headers show
Series apply unary op to both sides of (vec_cond x cst1 cst2) | expand

Commit Message

Marc Glisse May 19, 2019, 4:21 p.m. UTC
Hello,

I noticed this one with BIT_NOT_EXPR a while ago. C++ testcase because I 
haven't looked at gimplefe yet...

2019-05-20  Marc Glisse  <marc.glisse@inria.fr>

gcc/
 	* match.pd (~(vec?cst1:cst2)); New transformation.

gcc/testsuite/
 	* g++.dg/tree-ssa/cprop-vcond.C

Comments

Richard Biener May 20, 2019, 8:08 a.m. UTC | #1
On Sun, May 19, 2019 at 6:22 PM Marc Glisse <marc.glisse@inria.fr> wrote:
>
> Hello,
>
> I noticed this one with BIT_NOT_EXPR a while ago. C++ testcase because I
> haven't looked at gimplefe yet...

OK.  I wonder if there's any issue with -ftrapv and negate/abs of
INT_MIN containing
vectors (we don't implement -ftrapv optabs for vectors of course...).
We at least
avoid folding those cases I think and sanitizers then may see those
unconditionally?

Richard.

> 2019-05-20  Marc Glisse  <marc.glisse@inria.fr>
>
> gcc/
>         * match.pd (~(vec?cst1:cst2)); New transformation.
>
> gcc/testsuite/
>         * g++.dg/tree-ssa/cprop-vcond.C
>
> --
> Marc Glisse
Marc Glisse May 20, 2019, 1:37 p.m. UTC | #2
On Mon, 20 May 2019, Richard Biener wrote:

> On Sun, May 19, 2019 at 6:22 PM Marc Glisse <marc.glisse@inria.fr> wrote:
>>
>> Hello,
>>
>> I noticed this one with BIT_NOT_EXPR a while ago. C++ testcase because I
>> haven't looked at gimplefe yet...
>
> OK.  I wonder if there's any issue with -ftrapv and negate/abs of
> INT_MIN containing
> vectors (we don't implement -ftrapv optabs for vectors of course...).
> We at least
> avoid folding those cases I think and sanitizers then may see those
> unconditionally?

You are right. I had thought of floating point operations (say sqrt) where 
we might not fold because of the rounding mode, but the issue is more 
general. I believe I should really do what I was lazily trying to avoid: 
fold both operations and only perform the simplification if both returned 
constants.
Marc Glisse May 29, 2019, 10:02 p.m. UTC | #3
On Mon, 20 May 2019, Richard Biener wrote:

> On Sun, May 19, 2019 at 6:22 PM Marc Glisse <marc.glisse@inria.fr> wrote:
>>
>> Hello,
>>
>> I noticed this one with BIT_NOT_EXPR a while ago. C++ testcase because I
>> haven't looked at gimplefe yet...
>
> OK.  I wonder if there's any issue with -ftrapv and negate/abs of 
> INT_MIN containing vectors (we don't implement -ftrapv optabs for 
> vectors of course...).
> We at least avoid folding those cases I think and sanitizers then may 
> see those unconditionally?

The new version of the patch checks that both sides can indeed be folded
to constants before doing the transformation. We should have the same
for functions like sqrt, but fold_const_call does not seem to handle
vectors :-(

@@ -2911,20 +2913,35 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
     @2)
    (if (!VOID_TYPE_P (TREE_TYPE (@1)) || VOID_TYPE_P (type))
     @1)))
  (simplify
   (vec_cond VECTOR_CST@0 @1 @2)
   (if (integer_all_onesp (@0))
    @1
    (if (integer_zerop (@0))
     @2)))

+/* Sink unary operations to constant branches, but only if we do fold it to
+   constants.  */
+(for op (negate bit_not abs absu)
+ (simplify
+  (op (vec_cond @0 VECTOR_CST@1 VECTOR_CST@2))
+  (with
+   {
+     tree cst1, cst2;
+     cst1 = const_unop (op, type, @1);
+     if (cst1)
+       cst2 = const_unop (op, type, @2);
+   }
+   (if (cst1 && cst2)
+    (vec_cond @0 { cst1; } { cst2; })))))
+
  /* Simplification moved from fold_cond_expr_with_comparison.  It may also
     be extended.  */
  /* This pattern implements two kinds simplification:

     Case 1)
     (cond (cmp (convert1? x) c1) (convert2? x) c2) -> (minmax (x c)) if:
       1) Conversions are type widening from smaller type.
       2) Const c1 equals to c2 after canonicalizing comparison.
       3) Comparison has tree code LT, LE, GT or GE.
     This specific pattern is needed when (cmp (convert x) c) may not


>> 2019-05-20  Marc Glisse  <marc.glisse@inria.fr>
>>
>> gcc/
>>         * match.pd (~(vec?cst1:cst2)); New transformation.
>>
>> gcc/testsuite/
>>         * g++.dg/tree-ssa/cprop-vcond.C
Richard Biener May 31, 2019, 8:38 a.m. UTC | #4
On Thu, May 30, 2019 at 12:02 AM Marc Glisse <marc.glisse@inria.fr> wrote:
>
> On Mon, 20 May 2019, Richard Biener wrote:
>
> > On Sun, May 19, 2019 at 6:22 PM Marc Glisse <marc.glisse@inria.fr> wrote:
> >>
> >> Hello,
> >>
> >> I noticed this one with BIT_NOT_EXPR a while ago. C++ testcase because I
> >> haven't looked at gimplefe yet...
> >
> > OK.  I wonder if there's any issue with -ftrapv and negate/abs of
> > INT_MIN containing vectors (we don't implement -ftrapv optabs for
> > vectors of course...).
> > We at least avoid folding those cases I think and sanitizers then may
> > see those unconditionally?
>
> The new version of the patch checks that both sides can indeed be folded
> to constants before doing the transformation. We should have the same
> for functions like sqrt, but fold_const_call does not seem to handle
> vectors :-(

Looks good to me.  Btw, no reason to not extend fold_const_call to handle
the vector case ;)

Richard.

> @@ -2911,20 +2913,35 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>      @2)
>     (if (!VOID_TYPE_P (TREE_TYPE (@1)) || VOID_TYPE_P (type))
>      @1)))
>   (simplify
>    (vec_cond VECTOR_CST@0 @1 @2)
>    (if (integer_all_onesp (@0))
>     @1
>     (if (integer_zerop (@0))
>      @2)))
>
> +/* Sink unary operations to constant branches, but only if we do fold it to
> +   constants.  */
> +(for op (negate bit_not abs absu)
> + (simplify
> +  (op (vec_cond @0 VECTOR_CST@1 VECTOR_CST@2))
> +  (with
> +   {
> +     tree cst1, cst2;
> +     cst1 = const_unop (op, type, @1);
> +     if (cst1)
> +       cst2 = const_unop (op, type, @2);
> +   }
> +   (if (cst1 && cst2)
> +    (vec_cond @0 { cst1; } { cst2; })))))
> +
>   /* Simplification moved from fold_cond_expr_with_comparison.  It may also
>      be extended.  */
>   /* This pattern implements two kinds simplification:
>
>      Case 1)
>      (cond (cmp (convert1? x) c1) (convert2? x) c2) -> (minmax (x c)) if:
>        1) Conversions are type widening from smaller type.
>        2) Const c1 equals to c2 after canonicalizing comparison.
>        3) Comparison has tree code LT, LE, GT or GE.
>      This specific pattern is needed when (cmp (convert x) c) may not
>
>
> >> 2019-05-20  Marc Glisse  <marc.glisse@inria.fr>
> >>
> >> gcc/
> >>         * match.pd (~(vec?cst1:cst2)); New transformation.
> >>
> >> gcc/testsuite/
> >>         * g++.dg/tree-ssa/cprop-vcond.C
>
> --
> Marc Glisse
diff mbox series

Patch

Index: gcc/match.pd
===================================================================
--- gcc/match.pd	(revision 271376)
+++ gcc/match.pd	(working copy)
@@ -2911,20 +2913,26 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
    @2)
   (if (!VOID_TYPE_P (TREE_TYPE (@1)) || VOID_TYPE_P (type))
    @1)))
 (simplify
  (vec_cond VECTOR_CST@0 @1 @2)
  (if (integer_all_onesp (@0))
   @1
   (if (integer_zerop (@0))
    @2)))
 
+/* Sink unary operations to constant branches.  */
+(for op (negate bit_not abs absu)
+ (simplify
+  (op (vec_cond @0 VECTOR_CST@1 VECTOR_CST@2))
+  (vec_cond @0 (op @1) (op @2))))
+
 /* Simplification moved from fold_cond_expr_with_comparison.  It may also
    be extended.  */
 /* This pattern implements two kinds simplification:
 
    Case 1)
    (cond (cmp (convert1? x) c1) (convert2? x) c2) -> (minmax (x c)) if:
      1) Conversions are type widening from smaller type.
      2) Const c1 equals to c2 after canonicalizing comparison.
      3) Comparison has tree code LT, LE, GT or GE.
    This specific pattern is needed when (cmp (convert x) c) may not
Index: gcc/testsuite/g++.dg/tree-ssa/cprop-vcond.C
===================================================================
--- gcc/testsuite/g++.dg/tree-ssa/cprop-vcond.C	(nonexistent)
+++ gcc/testsuite/g++.dg/tree-ssa/cprop-vcond.C	(working copy)
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-forwprop1-raw" } */
+
+typedef long vec __attribute__((vector_size(2*sizeof(long))));
+void f(vec*v){
+  vec t = { 5, 16 };
+  vec f = { 27, -11 };
+  vec r = *v ? t : f;
+  *v = -r;
+}
+
+/* { dg-final { scan-tree-dump-not "negate_expr" "forwprop1" } } */