PR tree-optimization/103216: optimize some A ? (b op CST) : b into b op (A?CST:CST2)

Message ID 1636934929-842-1-git-send-email-apinski@marvell.com
State New
Headers
Series PR tree-optimization/103216: optimize some A ? (b op CST) : b into b op (A?CST:CST2) |

Commit Message

Li, Pan2 via Gcc-patches Nov. 15, 2021, 12:08 a.m. UTC
  From: Andrew Pinski <apinski@marvell.com>

For this PR, we have:
  if (d_5 < 0)
    goto <bb 3>; [INV]
  else
    goto <bb 4>; [INV]

  <bb 3> :
  v_7 = c_4 | -128;

  <bb 4> :
  # v_1 = PHI <c_4(2), v_7(3)>

Which PHI-OPT will try to simplify
"(d_5 < 0) ? (c_4 | -128) : c_4" which is not handled currently.
This adds a few patterns which allows to try to see if (a ? CST : CST1)
where CST1 is either 0, 1 or -1 depending on the operator.
Note to optimize this case always, we should check to make sure that
the a?CST:CST1 gets simplified to not include the conditional expression.
The ! flag does not work as we want to have more simplifcations than just
when we simplify it to a leaf node (SSA_NAME or CONSTANT). This adds a new
flag ^ to genmatch which says the simplification should happen but not down
to the same kind of node.
We could allow this for !GIMPLE and use fold_* rather than fold_buildN but I
didn't see any use of it for now.

Also all of these patterns need to be done late as other optimizations can be
done without them.

OK? Bootstrapped and tested on x86_64 with no regressions.

gcc/ChangeLog:

	* doc/match-and-simplify.texi: Document ^ flag.
	* genmatch.c (expr::expr): Add Setting of force_simplify.
	(expr): Add force_simplify field.
	(expr::gen_transform): Add support for force_simplify field.
	(parser::parse_expr): Add parsing of ^ flag for the expr.
	* match.pd: New patterns to optimize "a ? (b op CST) : b".
---
 gcc/doc/match-and-simplify.texi | 16 +++++++++++++
 gcc/genmatch.c                  | 35 ++++++++++++++++++++++++++--
 gcc/match.pd                    | 41 +++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 2 deletions(-)
  

Comments

Richard Biener Nov. 16, 2021, 10:17 a.m. UTC | #1
On Mon, Nov 15, 2021 at 1:09 AM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apinski@marvell.com>
>
> For this PR, we have:
>   if (d_5 < 0)
>     goto <bb 3>; [INV]
>   else
>     goto <bb 4>; [INV]
>
>   <bb 3> :
>   v_7 = c_4 | -128;
>
>   <bb 4> :
>   # v_1 = PHI <c_4(2), v_7(3)>
>
> Which PHI-OPT will try to simplify
> "(d_5 < 0) ? (c_4 | -128) : c_4" which is not handled currently.
> This adds a few patterns which allows to try to see if (a ? CST : CST1)
> where CST1 is either 0, 1 or -1 depending on the operator.
> Note to optimize this case always, we should check to make sure that
> the a?CST:CST1 gets simplified to not include the conditional expression.
> The ! flag does not work as we want to have more simplifcations than just
> when we simplify it to a leaf node (SSA_NAME or CONSTANT). This adds a new
> flag ^ to genmatch which says the simplification should happen but not down
> to the same kind of node.
> We could allow this for !GIMPLE and use fold_* rather than fold_buildN but I
> didn't see any use of it for now.
>
> Also all of these patterns need to be done late as other optimizations can be
> done without them.
>
> OK? Bootstrapped and tested on x86_64 with no regressions.
>
> gcc/ChangeLog:
>
>         * doc/match-and-simplify.texi: Document ^ flag.
>         * genmatch.c (expr::expr): Add Setting of force_simplify.
>         (expr): Add force_simplify field.
>         (expr::gen_transform): Add support for force_simplify field.
>         (parser::parse_expr): Add parsing of ^ flag for the expr.
>         * match.pd: New patterns to optimize "a ? (b op CST) : b".
> ---
>  gcc/doc/match-and-simplify.texi | 16 +++++++++++++
>  gcc/genmatch.c                  | 35 ++++++++++++++++++++++++++--
>  gcc/match.pd                    | 41 +++++++++++++++++++++++++++++++++
>  3 files changed, 90 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/doc/match-and-simplify.texi b/gcc/doc/match-and-simplify.texi
> index e7e5a4f7299..4e3407c0263 100644
> --- a/gcc/doc/match-and-simplify.texi
> +++ b/gcc/doc/match-and-simplify.texi
> @@ -377,6 +377,22 @@ of the @code{vec_cond} expression but only if the actual plus
>  operations both simplify.  Note this is currently only supported
>  for code generation targeting @code{GIMPLE}.
>
> +Another modifier for generated expressions is @code{^} which
> +tells the machinery to only consider the simplification in case
> +the marked expression simplified away from the original code.
> +Consider for example
> +
> +@smallexample
> +(simplify
> + (cond @@0 (plus:s @@1 INTEGER_CST@@2) @@1)
> + (plus @@1 (cond^ @@0 @@2 @{ build_zero_cst (type); @})))
> +@end smallexample
> +
> +which moves the inner @code{plus} operation to the outside of the
> +@code{cond} expression but only if the actual cond operation simplify
> +wayaway from cond.  Note this is currently only supported for code

s/wayaway/away/

> +generation targeting @code{GIMPLE}.
> +
>  As intermediate conversions are often optional there is a way to
>  avoid the need to repeat patterns both with and without such
>  conversions.  Namely you can mark a conversion as being optional
> diff --git a/gcc/genmatch.c b/gcc/genmatch.c
> index 95248455ec5..2dca1141df6 100644
> --- a/gcc/genmatch.c
> +++ b/gcc/genmatch.c
> @@ -698,12 +698,13 @@ public:
>      : operand (OP_EXPR, loc), operation (operation_),
>        ops (vNULL), expr_type (NULL), is_commutative (is_commutative_),
>        is_generic (false), force_single_use (false), force_leaf (false),
> -      opt_grp (0) {}
> +      force_simplify(false), opt_grp (0) {}
>    expr (expr *e)
>      : operand (OP_EXPR, e->location), operation (e->operation),
>        ops (vNULL), expr_type (e->expr_type), is_commutative (e->is_commutative),
>        is_generic (e->is_generic), force_single_use (e->force_single_use),
> -      force_leaf (e->force_leaf), opt_grp (e->opt_grp) {}
> +      force_leaf (e->force_leaf), force_simplify(e->force_simplify),
> +      opt_grp (e->opt_grp) {}
>    void append_op (operand *op) { ops.safe_push (op); }
>    /* The operator and its operands.  */
>    id_base *operation;
> @@ -721,6 +722,9 @@ public:
>    /* Whether in the result expression this should be a leaf node
>       with any children simplified down to simple operands.  */
>    bool force_leaf;
> +  /* Whether in the result expression this should be a node
> +     with any children simplified down not to use the original operator.  */
> +  bool force_simplify;
>    /* If non-zero, the group for optional handling.  */
>    unsigned char opt_grp;
>    virtual void gen_transform (FILE *f, int, const char *, bool, int,
> @@ -2527,6 +2531,17 @@ expr::gen_transform (FILE *f, int indent, const char *dest, bool gimple,
>         fprintf (f, ", _o%d[%u]", depth, i);
>        fprintf (f, ");\n");
>        fprintf_indent (f, indent, "tem_op.resimplify (lseq, valueize);\n");

I wonder if with force_simplify we should pass NULL as lseq to resimplify?
That is, should we allow (plus^ (convert @0) @1) to simplify to
(convert (plus @0 @1')) if @1 is trivially convertible for example?
"away from" is a bit of a grey area here.  I see you want a COND_EXPR
to become a non-COND_EXPR

> +      if (force_simplify)
> +       {
> +         fprintf_indent (f, indent, "if (tem_op.code.is_tree_code ())\n");
> +         fprintf_indent (f, indent, "  {\n");
> +         indent+=4;
> +         fprintf_indent (f, indent, "if (((tree_code)tem_op.code) == %s)\n",
> +                         opr_name);
> +         fprintf_indent (f, indent, "  goto %s;\n", fail_label);
> +         indent-=4;
> +         fprintf_indent (f, indent, "  }\n");
> +       }
>        fprintf_indent (f, indent,
>                       "_r%d = maybe_push_res_to_seq (&tem_op, %s);\n", depth,
>                       !force_leaf ? "lseq" : "NULL");
> @@ -4304,6 +4319,22 @@ parser::parse_expr ()
>        e->force_leaf = true;
>      }
>
> +  if (!parsing_match_operand
> +      && token->type == CPP_XOR
> +      && !(token->flags & PREV_WHITE))
> +    {
> +      if (!gimple)
> +       fatal_at (token, "forcing simplification is not supported for GENERIC");
> +      if (e->force_leaf)
> +       fatal_at (token, "forcing simplification and forcing to a leaf is not "
> +                 "supported");
> +      if (e->operation->kind != id_base::CODE)
> +       fatal_at (token, "forcing simplification is not supported on non-CODE "
> +                 "operations");
> +      eat_token (CPP_XOR);
> +      e->force_simplify = true;
> +    }
> +
>    if (token->type == CPP_COLON
>        && !(token->flags & PREV_WHITE))
>      {
> diff --git a/gcc/match.pd b/gcc/match.pd
> index df31964e02f..c66e918bb65 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -4186,6 +4186,47 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  )
>  #endif
>
> +#if GIMPLE
> +(if (canonicalize_math_p ())
> +/* These patterns are mostly used by PHIOPT to move some operations outside of
> +   the if statements. They should be done late because it gives jump threading
> +   and few other passes to reduce what is going on.  */

but canonicalize_math_p is _early_ ...?

> +/* a ? x op CST : x -> x op (a ? CST : 0) if (a ? CST : 0) can be simplified. */
> + (for op (plus minus bit_ior bit_xor lshift rshift lrotate rrotate)
> +  (simplify
> +   (cond @0 (op:s @1 INTEGER_CST@2) @1)
> +   (op @1 (cond^ @0 @2 { build_zero_cst (type); }))
> +  )
> +  (simplify
> +   (cond @0 @1 (op:s @1 INTEGER_CST@2))
> +   (op @1 (cond^ @0 { build_zero_cst (type); } @2))
> +  )
> + )
> +/* a ? x op CST : x -> x op (a ? CST : 1) if (a ? CST : 1) can be simplified. */
> + (for op (mult trunc_div ceil_div floor_div)
> +  (simplify
> +   (cond @0 (op:s @1 INTEGER_CST@2) @1)
> +   (op @1 (cond^ @0 @2 { build_one_cst (type); }))
> +  )
> +  (simplify
> +   (cond @0 @1 (op:s @1 INTEGER_CST@2))
> +   (op @1 (cond^ @0 { build_one_cst (type); } @2))
> +  )
> + )
> +/* a ? x op CST : x -> x op (a ? CST : -1) if (a ? CST : -1) can be simplified. */
> + (for op (bit_and)
> +  (simplify
> +   (cond @0 (op @1 INTEGER_CST@2) @1)
> +   (op @1 (cond^ @0 @2 { build_all_ones_cst (type); }))
> +   )
> +  (simplify
> +   (cond @0 @1 (op @1 INTEGER_CST@2))
> +   (op @1 (cond^ @0 { build_all_ones_cst (type); } @2))
> +  )
> + )
> +)

I wonder if phiopt could simply try simplifying (cond ...) itself,
thus programmatically do what you do with ^ and those extra patterns.

Richard.

> +#endif
> +
>  /* Simplification moved from fold_cond_expr_with_comparison.  It may also
>     be extended.  */
>  /* This pattern implements two kinds simplification:
> --
> 2.17.1
>
  

Patch

diff --git a/gcc/doc/match-and-simplify.texi b/gcc/doc/match-and-simplify.texi
index e7e5a4f7299..4e3407c0263 100644
--- a/gcc/doc/match-and-simplify.texi
+++ b/gcc/doc/match-and-simplify.texi
@@ -377,6 +377,22 @@  of the @code{vec_cond} expression but only if the actual plus
 operations both simplify.  Note this is currently only supported
 for code generation targeting @code{GIMPLE}.
 
+Another modifier for generated expressions is @code{^} which
+tells the machinery to only consider the simplification in case
+the marked expression simplified away from the original code.
+Consider for example
+
+@smallexample
+(simplify
+ (cond @@0 (plus:s @@1 INTEGER_CST@@2) @@1)
+ (plus @@1 (cond^ @@0 @@2 @{ build_zero_cst (type); @})))
+@end smallexample
+
+which moves the inner @code{plus} operation to the outside of the
+@code{cond} expression but only if the actual cond operation simplify
+wayaway from cond.  Note this is currently only supported for code
+generation targeting @code{GIMPLE}.
+
 As intermediate conversions are often optional there is a way to
 avoid the need to repeat patterns both with and without such
 conversions.  Namely you can mark a conversion as being optional
diff --git a/gcc/genmatch.c b/gcc/genmatch.c
index 95248455ec5..2dca1141df6 100644
--- a/gcc/genmatch.c
+++ b/gcc/genmatch.c
@@ -698,12 +698,13 @@  public:
     : operand (OP_EXPR, loc), operation (operation_),
       ops (vNULL), expr_type (NULL), is_commutative (is_commutative_),
       is_generic (false), force_single_use (false), force_leaf (false),
-      opt_grp (0) {}
+      force_simplify(false), opt_grp (0) {}
   expr (expr *e)
     : operand (OP_EXPR, e->location), operation (e->operation),
       ops (vNULL), expr_type (e->expr_type), is_commutative (e->is_commutative),
       is_generic (e->is_generic), force_single_use (e->force_single_use),
-      force_leaf (e->force_leaf), opt_grp (e->opt_grp) {}
+      force_leaf (e->force_leaf), force_simplify(e->force_simplify),
+      opt_grp (e->opt_grp) {}
   void append_op (operand *op) { ops.safe_push (op); }
   /* The operator and its operands.  */
   id_base *operation;
@@ -721,6 +722,9 @@  public:
   /* Whether in the result expression this should be a leaf node
      with any children simplified down to simple operands.  */
   bool force_leaf;
+  /* Whether in the result expression this should be a node
+     with any children simplified down not to use the original operator.  */
+  bool force_simplify;
   /* If non-zero, the group for optional handling.  */
   unsigned char opt_grp;
   virtual void gen_transform (FILE *f, int, const char *, bool, int,
@@ -2527,6 +2531,17 @@  expr::gen_transform (FILE *f, int indent, const char *dest, bool gimple,
 	fprintf (f, ", _o%d[%u]", depth, i);
       fprintf (f, ");\n");
       fprintf_indent (f, indent, "tem_op.resimplify (lseq, valueize);\n");
+      if (force_simplify)
+	{
+	  fprintf_indent (f, indent, "if (tem_op.code.is_tree_code ())\n");
+	  fprintf_indent (f, indent, "  {\n");
+	  indent+=4;
+	  fprintf_indent (f, indent, "if (((tree_code)tem_op.code) == %s)\n",
+			  opr_name);
+	  fprintf_indent (f, indent, "  goto %s;\n", fail_label);
+	  indent-=4;
+	  fprintf_indent (f, indent, "  }\n");
+	}
       fprintf_indent (f, indent,
 		      "_r%d = maybe_push_res_to_seq (&tem_op, %s);\n", depth,
 		      !force_leaf ? "lseq" : "NULL");
@@ -4304,6 +4319,22 @@  parser::parse_expr ()
       e->force_leaf = true;
     }
 
+  if (!parsing_match_operand
+      && token->type == CPP_XOR
+      && !(token->flags & PREV_WHITE))
+    {
+      if (!gimple)
+	fatal_at (token, "forcing simplification is not supported for GENERIC");
+      if (e->force_leaf)
+	fatal_at (token, "forcing simplification and forcing to a leaf is not "
+		  "supported");
+      if (e->operation->kind != id_base::CODE)
+	fatal_at (token, "forcing simplification is not supported on non-CODE "
+		  "operations");
+      eat_token (CPP_XOR);
+      e->force_simplify = true;
+    }
+
   if (token->type == CPP_COLON
       && !(token->flags & PREV_WHITE))
     {
diff --git a/gcc/match.pd b/gcc/match.pd
index df31964e02f..c66e918bb65 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4186,6 +4186,47 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 )
 #endif
 
+#if GIMPLE
+(if (canonicalize_math_p ())
+/* These patterns are mostly used by PHIOPT to move some operations outside of
+   the if statements. They should be done late because it gives jump threading
+   and few other passes to reduce what is going on.  */
+/* a ? x op CST : x -> x op (a ? CST : 0) if (a ? CST : 0) can be simplified. */
+ (for op (plus minus bit_ior bit_xor lshift rshift lrotate rrotate)
+  (simplify
+   (cond @0 (op:s @1 INTEGER_CST@2) @1)
+   (op @1 (cond^ @0 @2 { build_zero_cst (type); }))
+  )
+  (simplify
+   (cond @0 @1 (op:s @1 INTEGER_CST@2))
+   (op @1 (cond^ @0 { build_zero_cst (type); } @2))
+  )
+ )
+/* a ? x op CST : x -> x op (a ? CST : 1) if (a ? CST : 1) can be simplified. */
+ (for op (mult trunc_div ceil_div floor_div)
+  (simplify
+   (cond @0 (op:s @1 INTEGER_CST@2) @1)
+   (op @1 (cond^ @0 @2 { build_one_cst (type); }))
+  )
+  (simplify
+   (cond @0 @1 (op:s @1 INTEGER_CST@2))
+   (op @1 (cond^ @0 { build_one_cst (type); } @2))
+  )
+ )
+/* a ? x op CST : x -> x op (a ? CST : -1) if (a ? CST : -1) can be simplified. */
+ (for op (bit_and)
+  (simplify
+   (cond @0 (op @1 INTEGER_CST@2) @1)
+   (op @1 (cond^ @0 @2 { build_all_ones_cst (type); }))
+   )
+  (simplify
+   (cond @0 @1 (op @1 INTEGER_CST@2))
+   (op @1 (cond^ @0 { build_all_ones_cst (type); } @2))
+  )
+ )
+)
+#endif
+
 /* Simplification moved from fold_cond_expr_with_comparison.  It may also
    be extended.  */
 /* This pattern implements two kinds simplification: