Fix up *COND_EXPR *trap* handling (PR middle-end/92063)

Hi!

As mentioned in the PR and on IRC, tree_could_trap_p is described
as taking GIMPLE expressions only, but in fact we rely on it not crashing
when feeded GENERIC, just for GENERIC it will not handle expressions
recursively and we have generic_expr_could_trap_p for that that calls
tree_could_trap_p recursively.
The addition of != COND_EXPR and != VEC_COND_EXPR assert to
operation_could_trap_p broke this, because if GENERIC with COND_EXPR
being first argument (condition) of another COND_EXPR is fed to
tree_could_trap_p, it now ICEs.

The following patch fixes it by:
1) in tree_could_trap_p return false for {,VEC_}COND_EXPR rather than
just recursing on the first argument.  For GIMPLE, we shouldn't be called
with {,VEC_}COND_EXPR, because those are ternary rhs and thus 3 arguments,
not one.  For GENERIC, we can, but we also need to recurse and the recursion
should discover that the comparison may trap.
2) in simple_operand_p_2 which calls tree_could_trap_p on GENERIC, this is
changed to generic_expr_could_trap_p so that it actually recurses and tests
subexpressions too
3) in operation_could_trap_helper_p signals that *COND_EXPR is not handled
and it doesn't care about whether *COND_EXPR is floating point or not.
This change is for sccvn, which calls operation_could_trap_helper_p
and then, if not handled, calls tree_could_trap_p on the operands.
Without the first hunk, *COND_EXPR is handled by the default handling,
which says that fp_operation can trap (but *COND_EXPR with fp operands can't
in itself) and makes it unhandled otherwise, at which point we call
tree_could_trap_p on the condition, which is all that matters.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2019-10-12  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/92063
	* tree-eh.c (operation_could_trap_helper_p) <case COND_EXPR>
	<case VEC_COND_EXPR>: Return false with *handled = false.
	(tree_could_trap_p): For {,VEC_}COND_EXPR return false instead of
	recursing on the first operand.
	* fold-const.c (simple_operand_p_2): Use generic_expr_could_trap_p
	instead of tree_could_trap_p.
	* tree-ssa-sccvn.c (vn_nary_may_trap): Formatting fixes.

	* gcc.c-torture/compile/pr92063.c: New test.


	Jakub

On October 12, 2019 10:44:06 AM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>As mentioned in the PR and on IRC, tree_could_trap_p is described
>as taking GIMPLE expressions only, but in fact we rely on it not
>crashing
>when feeded GENERIC, just for GENERIC it will not handle expressions
>recursively and we have generic_expr_could_trap_p for that that calls
>tree_could_trap_p recursively.
>The addition of != COND_EXPR and != VEC_COND_EXPR assert to
>operation_could_trap_p broke this, because if GENERIC with COND_EXPR
>being first argument (condition) of another COND_EXPR is fed to
>tree_could_trap_p, it now ICEs.
>
>The following patch fixes it by:
>1) in tree_could_trap_p return false for {,VEC_}COND_EXPR rather than
>just recursing on the first argument.  For GIMPLE, we shouldn't be
>called
>with {,VEC_}COND_EXPR, because those are ternary rhs and thus 3
>arguments,
>not one.  For GENERIC, we can, but we also need to recurse and the
>recursion
>should discover that the comparison may trap.
>2) in simple_operand_p_2 which calls tree_could_trap_p on GENERIC, this
>is
>changed to generic_expr_could_trap_p so that it actually recurses and
>tests
>subexpressions too
>3) in operation_could_trap_helper_p signals that *COND_EXPR is not
>handled
>and it doesn't care about whether *COND_EXPR is floating point or not.
>This change is for sccvn, which calls operation_could_trap_helper_p
>and then, if not handled, calls tree_could_trap_p on the operands.
>Without the first hunk, *COND_EXPR is handled by the default handling,
>which says that fp_operation can trap (but *COND_EXPR with fp operands
>can't
>in itself) and makes it unhandled otherwise, at which point we call
>tree_could_trap_p on the condition, which is all that matters.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok. 

Thanks, 
Richard. 

>2019-10-12  Jakub Jelinek  <jakub@redhat.com>
>
>	PR middle-end/92063
>	* tree-eh.c (operation_could_trap_helper_p) <case COND_EXPR>
>	<case VEC_COND_EXPR>: Return false with *handled = false.
>	(tree_could_trap_p): For {,VEC_}COND_EXPR return false instead of
>	recursing on the first operand.
>	* fold-const.c (simple_operand_p_2): Use generic_expr_could_trap_p
>	instead of tree_could_trap_p.
>	* tree-ssa-sccvn.c (vn_nary_may_trap): Formatting fixes.
>
>	* gcc.c-torture/compile/pr92063.c: New test.
>
>--- gcc/tree-eh.c.jj	2019-10-07 17:30:31.028153702 +0200
>+++ gcc/tree-eh.c	2019-10-11 13:46:12.626811039 +0200
>@@ -2499,6 +2499,14 @@ operation_could_trap_helper_p (enum tree
>       /* Constructing an object cannot trap.  */
>       return false;
> 
>+    case COND_EXPR:
>+    case VEC_COND_EXPR:
>+      /* Whether *COND_EXPR can trap depends on whether the
>+	 first argument can trap, so signal it as not handled.
>+	 Whether lhs is floating or not doesn't matter.  */
>+      *handled = false;
>+      return false;
>+
>     default:
>       /* Any floating arithmetic may trap.  */
>       if (fp_operation && flag_trapping_math)
>@@ -2614,9 +2622,12 @@ tree_could_trap_p (tree expr)
>   if (!expr)
>     return false;
> 
>-  /* For COND_EXPR and VEC_COND_EXPR only the condition may trap.  */
>+  /* In COND_EXPR and VEC_COND_EXPR only the condition may trap, but
>+     they won't appear as operands in GIMPLE form, so this is just for
>the
>+     GENERIC uses where it needs to recurse on the operands and so
>+     *COND_EXPR itself doesn't trap.  */
>if (TREE_CODE (expr) == COND_EXPR || TREE_CODE (expr) == VEC_COND_EXPR)
>-    expr = TREE_OPERAND (expr, 0);
>+    return false;
> 
>   code = TREE_CODE (expr);
>   t = TREE_TYPE (expr);
>--- gcc/fold-const.c.jj	2019-10-09 10:27:12.578402783 +0200
>+++ gcc/fold-const.c	2019-10-11 12:29:53.426603712 +0200
>@@ -4447,8 +4447,7 @@ simple_operand_p_2 (tree exp)
> {
>   enum tree_code code;
> 
>-  if (TREE_SIDE_EFFECTS (exp)
>-      || tree_could_trap_p (exp))
>+  if (TREE_SIDE_EFFECTS (exp) || generic_expr_could_trap_p (exp))
>     return false;
> 
>   while (CONVERT_EXPR_P (exp))
>--- gcc/tree-ssa-sccvn.c.jj	2019-09-24 14:39:07.479465423 +0200
>+++ gcc/tree-ssa-sccvn.c	2019-10-11 13:21:47.437326054 +0200
>@@ -5100,18 +5100,15 @@ vn_nary_may_trap (vn_nary_op_t nary)
> 	  honor_nans = flag_trapping_math && !flag_finite_math_only;
> 	  honor_snans = flag_signaling_nans != 0;
> 	}
>-      else if (INTEGRAL_TYPE_P (type)
>-	       && TYPE_OVERFLOW_TRAPS (type))
>+      else if (INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_TRAPS (type))
> 	honor_trapv = true;
>     }
>   if (nary->length >= 2)
>     rhs2 = nary->op[1];
>   ret = operation_could_trap_helper_p (nary->opcode, fp_operation,
>-				       honor_trapv,
>-				       honor_nans, honor_snans, rhs2,
>-				       &handled);
>-  if (handled
>-      && ret)
>+				       honor_trapv, honor_nans, honor_snans,
>+				       rhs2, &handled);
>+  if (handled && ret)
>     return true;
> 
>   for (i = 0; i < nary->length; ++i)
>--- gcc/testsuite/gcc.c-torture/compile/pr92063.c.jj	2019-10-11
>11:51:47.959149238 +0200
>+++ gcc/testsuite/gcc.c-torture/compile/pr92063.c	2019-10-11
>11:55:16.675090011 +0200
>@@ -0,0 +1,7 @@
>+/* PR middle-end/92063 */
>+
>+int
>+foo (int a, int b, int *c, short *d)
>+{
>+  return (c[0] ? b : 0) == 'y' && ((a ? d[0] : c[0]) ? b : 0) == 'c';
>+}
>
>	Jakub