[v4,1/7] Allow COND_EXPR and VEC_COND_EXPR condtions to trap

Message ID 20191001132709.87257-2-iii@linux.ibm.com
State New
Headers show
Series
  • S/390: Use signaling FP comparison instructions
Related show

Commit Message

Ilya Leoshkevich Oct. 1, 2019, 1:27 p.m.
Right now gimplifier does not allow VEC_COND_EXPR's condition to trap
and introduces a temporary if this could happen, for example, generating

  _5 = _4 > { 2.0e+0, 2.0e+0, 2.0e+0, 2.0e+0 };
  _6 = VEC_COND_EXPR <_5, { -1, -1, -1, -1 }, { 0, 0, 0, 0 }>;

from GENERIC

  VEC_COND_EXPR < (*b > { 2.0e+0, 2.0e+0, 2.0e+0, 2.0e+0 }) ,
                  { -1, -1, -1, -1 } ,
		  { 0, 0, 0, 0 } >

This is not necessary and makes the resulting GIMPLE harder to analyze.
Change the gimplifier so as to allow COND_EXPR and VEC_COND_EXPR
conditions to trap.

This patch takes special care to avoid introducing trapping comparisons
in GIMPLE_COND.  They are not allowed, because they would require 3
outgoing edges (then, else and EH), which is awkward to say the least.
Therefore, computations of such conditions should live in their own basic
blocks.

gcc/ChangeLog:

2019-09-03  Ilya Leoshkevich  <iii@linux.ibm.com>

	PR target/77918
	* gimple-expr.c (gimple_cond_get_ops_from_tree): Assert that the
	caller passes a non-trapping condition.
	(is_gimple_condexpr): Allow trapping conditions.
	(is_gimple_condexpr_1): New helper function.
	(is_gimple_condexpr_for_cond): New function, acts like old
	is_gimple_condexpr.
	* gimple-expr.h (is_gimple_condexpr_for_cond): New function.
	* gimple.c (gimple_could_trap_p_1): Handle COND_EXPR and
	VEC_COND_EXPR. Fix an issue with statements like i = (fp < 1.).
	* gimplify.c (gimplify_cond_expr): Use
	is_gimple_condexpr_for_cond.
	(gimplify_expr): Allow is_gimple_condexpr_for_cond.
	* tree-eh.c (operation_could_trap_p): Assert on COND_EXPR and
	VEC_COND_EXPR.
	(tree_could_trap_p): Handle COND_EXPR and VEC_COND_EXPR.
	* tree-ssa-forwprop.c (forward_propagate_into_gimple_cond): Use
	is_gimple_condexpr_for_cond, remove pointless tmp check
	(forward_propagate_into_cond): Remove pointless tmp check.
---
 gcc/gimple-expr.c       | 25 +++++++++++++++++++++----
 gcc/gimple-expr.h       |  1 +
 gcc/gimple.c            | 14 +++++++++++++-
 gcc/gimplify.c          |  5 +++--
 gcc/tree-eh.c           |  8 ++++++++
 gcc/tree-ssa-forwprop.c |  7 ++++---
 6 files changed, 50 insertions(+), 10 deletions(-)

-- 
2.23.0

Comments

Richard Biener Oct. 7, 2019, 10:02 a.m. | #1
On Tue, Oct 1, 2019 at 3:27 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>

> Right now gimplifier does not allow VEC_COND_EXPR's condition to trap

> and introduces a temporary if this could happen, for example, generating

>

>   _5 = _4 > { 2.0e+0, 2.0e+0, 2.0e+0, 2.0e+0 };

>   _6 = VEC_COND_EXPR <_5, { -1, -1, -1, -1 }, { 0, 0, 0, 0 }>;

>

> from GENERIC

>

>   VEC_COND_EXPR < (*b > { 2.0e+0, 2.0e+0, 2.0e+0, 2.0e+0 }) ,

>                   { -1, -1, -1, -1 } ,

>                   { 0, 0, 0, 0 } >

>

> This is not necessary and makes the resulting GIMPLE harder to analyze.

> Change the gimplifier so as to allow COND_EXPR and VEC_COND_EXPR

> conditions to trap.

>

> This patch takes special care to avoid introducing trapping comparisons

> in GIMPLE_COND.  They are not allowed, because they would require 3

> outgoing edges (then, else and EH), which is awkward to say the least.

> Therefore, computations of such conditions should live in their own basic

> blocks.


OK.  This can go in independently of the other changes in this series.

Thanks and sorry for the delay.
Richard.

> gcc/ChangeLog:

>

> 2019-09-03  Ilya Leoshkevich  <iii@linux.ibm.com>

>

>         PR target/77918

>         * gimple-expr.c (gimple_cond_get_ops_from_tree): Assert that the

>         caller passes a non-trapping condition.

>         (is_gimple_condexpr): Allow trapping conditions.

>         (is_gimple_condexpr_1): New helper function.

>         (is_gimple_condexpr_for_cond): New function, acts like old

>         is_gimple_condexpr.

>         * gimple-expr.h (is_gimple_condexpr_for_cond): New function.

>         * gimple.c (gimple_could_trap_p_1): Handle COND_EXPR and

>         VEC_COND_EXPR. Fix an issue with statements like i = (fp < 1.).

>         * gimplify.c (gimplify_cond_expr): Use

>         is_gimple_condexpr_for_cond.

>         (gimplify_expr): Allow is_gimple_condexpr_for_cond.

>         * tree-eh.c (operation_could_trap_p): Assert on COND_EXPR and

>         VEC_COND_EXPR.

>         (tree_could_trap_p): Handle COND_EXPR and VEC_COND_EXPR.

>         * tree-ssa-forwprop.c (forward_propagate_into_gimple_cond): Use

>         is_gimple_condexpr_for_cond, remove pointless tmp check

>         (forward_propagate_into_cond): Remove pointless tmp check.

> ---

>  gcc/gimple-expr.c       | 25 +++++++++++++++++++++----

>  gcc/gimple-expr.h       |  1 +

>  gcc/gimple.c            | 14 +++++++++++++-

>  gcc/gimplify.c          |  5 +++--

>  gcc/tree-eh.c           |  8 ++++++++

>  gcc/tree-ssa-forwprop.c |  7 ++++---

>  6 files changed, 50 insertions(+), 10 deletions(-)

>

> diff --git a/gcc/gimple-expr.c b/gcc/gimple-expr.c

> index 4082828e198..1738af186d7 100644

> --- a/gcc/gimple-expr.c

> +++ b/gcc/gimple-expr.c

> @@ -574,6 +574,7 @@ gimple_cond_get_ops_from_tree (tree cond, enum tree_code *code_p,

>               || TREE_CODE (cond) == TRUTH_NOT_EXPR

>               || is_gimple_min_invariant (cond)

>               || SSA_VAR_P (cond));

> +  gcc_checking_assert (!tree_could_throw_p (cond));

>

>    extract_ops_from_tree (cond, code_p, lhs_p, rhs_p);

>

> @@ -605,17 +606,33 @@ is_gimple_lvalue (tree t)

>           || TREE_CODE (t) == BIT_FIELD_REF);

>  }

>

> -/*  Return true if T is a GIMPLE condition.  */

> +/* Helper for is_gimple_condexpr and is_gimple_condexpr_for_cond.  */

>

> -bool

> -is_gimple_condexpr (tree t)

> +static bool

> +is_gimple_condexpr_1 (tree t, bool allow_traps)

>  {

>    return (is_gimple_val (t) || (COMPARISON_CLASS_P (t)

> -                               && !tree_could_throw_p (t)

> +                               && (allow_traps || !tree_could_throw_p (t))

>                                 && is_gimple_val (TREE_OPERAND (t, 0))

>                                 && is_gimple_val (TREE_OPERAND (t, 1))));

>  }

>

> +/* Return true if T is a GIMPLE condition.  */

> +

> +bool

> +is_gimple_condexpr (tree t)

> +{

> +  return is_gimple_condexpr_1 (t, true);

> +}

> +

> +/* Like is_gimple_condexpr, but does not allow T to trap.  */

> +

> +bool

> +is_gimple_condexpr_for_cond (tree t)

> +{

> +  return is_gimple_condexpr_1 (t, false);

> +}

> +

>  /* Return true if T is a gimple address.  */

>

>  bool

> diff --git a/gcc/gimple-expr.h b/gcc/gimple-expr.h

> index 1ad1432bd17..0925aeb0f57 100644

> --- a/gcc/gimple-expr.h

> +++ b/gcc/gimple-expr.h

> @@ -41,6 +41,7 @@ extern void gimple_cond_get_ops_from_tree (tree, enum tree_code *, tree *,

>                                            tree *);

>  extern bool is_gimple_lvalue (tree);

>  extern bool is_gimple_condexpr (tree);

> +extern bool is_gimple_condexpr_for_cond (tree);

>  extern bool is_gimple_address (const_tree);

>  extern bool is_gimple_invariant_address (const_tree);

>  extern bool is_gimple_ip_invariant_address (const_tree);

> diff --git a/gcc/gimple.c b/gcc/gimple.c

> index 8e828a5f169..a874c29454c 100644

> --- a/gcc/gimple.c

> +++ b/gcc/gimple.c

> @@ -2149,10 +2149,22 @@ gimple_could_trap_p_1 (gimple *s, bool include_mem, bool include_stores)

>        return false;

>

>      case GIMPLE_ASSIGN:

> -      t = gimple_expr_type (s);

>        op = gimple_assign_rhs_code (s);

> +

> +      /* For COND_EXPR and VEC_COND_EXPR only the condition may trap.  */

> +      if (op == COND_EXPR || op == VEC_COND_EXPR)

> +       return tree_could_trap_p (gimple_assign_rhs1 (s));

> +

> +      /* For comparisons we need to check rhs operand types instead of rhs type

> +         (which is BOOLEAN_TYPE).  */

> +      if (TREE_CODE_CLASS (op) == tcc_comparison)

> +       t = TREE_TYPE (gimple_assign_rhs1 (s));

> +      else

> +       t = gimple_expr_type (s);

> +

>        if (get_gimple_rhs_class (op) == GIMPLE_BINARY_RHS)

>         div = gimple_assign_rhs2 (s);

> +

>        return (operation_could_trap_p (op, FLOAT_TYPE_P (t),

>                                       (INTEGRAL_TYPE_P (t)

>                                        && TYPE_OVERFLOW_TRAPS (t)),

> diff --git a/gcc/gimplify.c b/gcc/gimplify.c

> index 88d6571976f..836706961f3 100644

> --- a/gcc/gimplify.c

> +++ b/gcc/gimplify.c

> @@ -4142,8 +4142,8 @@ gimplify_cond_expr (tree *expr_p, gimple_seq *pre_p, fallback_t fallback)

>    /* Now do the normal gimplification.  */

>

>    /* Gimplify condition.  */

> -  ret = gimplify_expr (&TREE_OPERAND (expr, 0), pre_p, NULL, is_gimple_condexpr,

> -                      fb_rvalue);

> +  ret = gimplify_expr (&TREE_OPERAND (expr, 0), pre_p, NULL,

> +                      is_gimple_condexpr_for_cond, fb_rvalue);

>    if (ret == GS_ERROR)

>      return GS_ERROR;

>    gcc_assert (TREE_OPERAND (expr, 0) != NULL_TREE);

> @@ -12976,6 +12976,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,

>    else if (gimple_test_f == is_gimple_val

>             || gimple_test_f == is_gimple_call_addr

>             || gimple_test_f == is_gimple_condexpr

> +          || gimple_test_f == is_gimple_condexpr_for_cond

>             || gimple_test_f == is_gimple_mem_rhs

>             || gimple_test_f == is_gimple_mem_rhs_or_call

>             || gimple_test_f == is_gimple_reg_rhs

> diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c

> index 5bb07e49d28..b3722ba8839 100644

> --- a/gcc/tree-eh.c

> +++ b/gcc/tree-eh.c

> @@ -2523,6 +2523,10 @@ operation_could_trap_p (enum tree_code op, bool fp_operation, bool honor_trapv,

>    bool honor_snans = fp_operation && flag_signaling_nans != 0;

>    bool handled;

>

> +  /* This function cannot tell whether or not COND_EXPR and VEC_COND_EXPR could

> +     trap, because that depends on the respective condition op.  */

> +  gcc_assert (op != COND_EXPR && op != VEC_COND_EXPR);

> +

>    if (TREE_CODE_CLASS (op) != tcc_comparison

>        && TREE_CODE_CLASS (op) != tcc_unary

>        && TREE_CODE_CLASS (op) != tcc_binary)

> @@ -2610,6 +2614,10 @@ tree_could_trap_p (tree expr)

>    if (!expr)

>      return false;

>

> +  /* For COND_EXPR and VEC_COND_EXPR only the condition may trap.  */

> +  if (TREE_CODE (expr) == COND_EXPR || TREE_CODE (expr) == VEC_COND_EXPR)

> +    expr = TREE_OPERAND (expr, 0);

> +

>    code = TREE_CODE (expr);

>    t = TREE_TYPE (expr);

>

> diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c

> index 221f140b356..0d1c066b354 100644

> --- a/gcc/tree-ssa-forwprop.c

> +++ b/gcc/tree-ssa-forwprop.c

> @@ -527,9 +527,10 @@ forward_propagate_into_gimple_cond (gcond *stmt)

>    tmp = forward_propagate_into_comparison_1 (stmt, code,

>                                              boolean_type_node,

>                                              rhs1, rhs2);

> -  if (tmp)

> +  if (tmp

> +      && is_gimple_condexpr_for_cond (tmp))

>      {

> -      if (dump_file && tmp)

> +      if (dump_file)

>         {

>           fprintf (dump_file, "  Replaced '");

>           print_gimple_expr (dump_file, stmt, 0);

> @@ -607,7 +608,7 @@ forward_propagate_into_cond (gimple_stmt_iterator *gsi_p)

>    if (tmp

>        && is_gimple_condexpr (tmp))

>      {

> -      if (dump_file && tmp)

> +      if (dump_file)

>         {

>           fprintf (dump_file, "  Replaced '");

>           print_generic_expr (dump_file, cond);

> --

> 2.23.0

>

Patch

diff --git a/gcc/gimple-expr.c b/gcc/gimple-expr.c
index 4082828e198..1738af186d7 100644
--- a/gcc/gimple-expr.c
+++ b/gcc/gimple-expr.c
@@ -574,6 +574,7 @@  gimple_cond_get_ops_from_tree (tree cond, enum tree_code *code_p,
 	      || TREE_CODE (cond) == TRUTH_NOT_EXPR
 	      || is_gimple_min_invariant (cond)
 	      || SSA_VAR_P (cond));
+  gcc_checking_assert (!tree_could_throw_p (cond));
 
   extract_ops_from_tree (cond, code_p, lhs_p, rhs_p);
 
@@ -605,17 +606,33 @@  is_gimple_lvalue (tree t)
 	  || TREE_CODE (t) == BIT_FIELD_REF);
 }
 
-/*  Return true if T is a GIMPLE condition.  */
+/* Helper for is_gimple_condexpr and is_gimple_condexpr_for_cond.  */
 
-bool
-is_gimple_condexpr (tree t)
+static bool
+is_gimple_condexpr_1 (tree t, bool allow_traps)
 {
   return (is_gimple_val (t) || (COMPARISON_CLASS_P (t)
-				&& !tree_could_throw_p (t)
+				&& (allow_traps || !tree_could_throw_p (t))
 				&& is_gimple_val (TREE_OPERAND (t, 0))
 				&& is_gimple_val (TREE_OPERAND (t, 1))));
 }
 
+/* Return true if T is a GIMPLE condition.  */
+
+bool
+is_gimple_condexpr (tree t)
+{
+  return is_gimple_condexpr_1 (t, true);
+}
+
+/* Like is_gimple_condexpr, but does not allow T to trap.  */
+
+bool
+is_gimple_condexpr_for_cond (tree t)
+{
+  return is_gimple_condexpr_1 (t, false);
+}
+
 /* Return true if T is a gimple address.  */
 
 bool
diff --git a/gcc/gimple-expr.h b/gcc/gimple-expr.h
index 1ad1432bd17..0925aeb0f57 100644
--- a/gcc/gimple-expr.h
+++ b/gcc/gimple-expr.h
@@ -41,6 +41,7 @@  extern void gimple_cond_get_ops_from_tree (tree, enum tree_code *, tree *,
 					   tree *);
 extern bool is_gimple_lvalue (tree);
 extern bool is_gimple_condexpr (tree);
+extern bool is_gimple_condexpr_for_cond (tree);
 extern bool is_gimple_address (const_tree);
 extern bool is_gimple_invariant_address (const_tree);
 extern bool is_gimple_ip_invariant_address (const_tree);
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 8e828a5f169..a874c29454c 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -2149,10 +2149,22 @@  gimple_could_trap_p_1 (gimple *s, bool include_mem, bool include_stores)
       return false;
 
     case GIMPLE_ASSIGN:
-      t = gimple_expr_type (s);
       op = gimple_assign_rhs_code (s);
+
+      /* For COND_EXPR and VEC_COND_EXPR only the condition may trap.  */
+      if (op == COND_EXPR || op == VEC_COND_EXPR)
+	return tree_could_trap_p (gimple_assign_rhs1 (s));
+
+      /* For comparisons we need to check rhs operand types instead of rhs type
+         (which is BOOLEAN_TYPE).  */
+      if (TREE_CODE_CLASS (op) == tcc_comparison)
+	t = TREE_TYPE (gimple_assign_rhs1 (s));
+      else
+	t = gimple_expr_type (s);
+
       if (get_gimple_rhs_class (op) == GIMPLE_BINARY_RHS)
 	div = gimple_assign_rhs2 (s);
+
       return (operation_could_trap_p (op, FLOAT_TYPE_P (t),
 				      (INTEGRAL_TYPE_P (t)
 				       && TYPE_OVERFLOW_TRAPS (t)),
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 88d6571976f..836706961f3 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -4142,8 +4142,8 @@  gimplify_cond_expr (tree *expr_p, gimple_seq *pre_p, fallback_t fallback)
   /* Now do the normal gimplification.  */
 
   /* Gimplify condition.  */
-  ret = gimplify_expr (&TREE_OPERAND (expr, 0), pre_p, NULL, is_gimple_condexpr,
-		       fb_rvalue);
+  ret = gimplify_expr (&TREE_OPERAND (expr, 0), pre_p, NULL,
+		       is_gimple_condexpr_for_cond, fb_rvalue);
   if (ret == GS_ERROR)
     return GS_ERROR;
   gcc_assert (TREE_OPERAND (expr, 0) != NULL_TREE);
@@ -12976,6 +12976,7 @@  gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
   else if (gimple_test_f == is_gimple_val
            || gimple_test_f == is_gimple_call_addr
            || gimple_test_f == is_gimple_condexpr
+	   || gimple_test_f == is_gimple_condexpr_for_cond
            || gimple_test_f == is_gimple_mem_rhs
            || gimple_test_f == is_gimple_mem_rhs_or_call
            || gimple_test_f == is_gimple_reg_rhs
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 5bb07e49d28..b3722ba8839 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -2523,6 +2523,10 @@  operation_could_trap_p (enum tree_code op, bool fp_operation, bool honor_trapv,
   bool honor_snans = fp_operation && flag_signaling_nans != 0;
   bool handled;
 
+  /* This function cannot tell whether or not COND_EXPR and VEC_COND_EXPR could
+     trap, because that depends on the respective condition op.  */
+  gcc_assert (op != COND_EXPR && op != VEC_COND_EXPR);
+
   if (TREE_CODE_CLASS (op) != tcc_comparison
       && TREE_CODE_CLASS (op) != tcc_unary
       && TREE_CODE_CLASS (op) != tcc_binary)
@@ -2610,6 +2614,10 @@  tree_could_trap_p (tree expr)
   if (!expr)
     return false;
 
+  /* For COND_EXPR and VEC_COND_EXPR only the condition may trap.  */
+  if (TREE_CODE (expr) == COND_EXPR || TREE_CODE (expr) == VEC_COND_EXPR)
+    expr = TREE_OPERAND (expr, 0);
+
   code = TREE_CODE (expr);
   t = TREE_TYPE (expr);
 
diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
index 221f140b356..0d1c066b354 100644
--- a/gcc/tree-ssa-forwprop.c
+++ b/gcc/tree-ssa-forwprop.c
@@ -527,9 +527,10 @@  forward_propagate_into_gimple_cond (gcond *stmt)
   tmp = forward_propagate_into_comparison_1 (stmt, code,
 					     boolean_type_node,
 					     rhs1, rhs2);
-  if (tmp)
+  if (tmp
+      && is_gimple_condexpr_for_cond (tmp))
     {
-      if (dump_file && tmp)
+      if (dump_file)
 	{
 	  fprintf (dump_file, "  Replaced '");
 	  print_gimple_expr (dump_file, stmt, 0);
@@ -607,7 +608,7 @@  forward_propagate_into_cond (gimple_stmt_iterator *gsi_p)
   if (tmp
       && is_gimple_condexpr (tmp))
     {
-      if (dump_file && tmp)
+      if (dump_file)
 	{
 	  fprintf (dump_file, "  Replaced '");
 	  print_generic_expr (dump_file, cond);