vect: Fix up lowering of TRUNC_MOD_EXPR by negative constant [PR94524]

Hi!

The first testcase below is miscompiled, because for the division part
of the lowering we canonicalize negative divisors to their absolute value
(similarly how expmed.c canonicalizes it), but when multiplying the division
result back by the VECTOR_CST, we use the original constant, which can
contain negative divisors.

Fixed by computing ABS_EXPR of the VECTOR_CST.  Unfortunately, fold-const.c
doesn't support const_unop (ABS_EXPR, VECTOR_CST) and I think it is too late
in GCC 10 cycle to add it now.

Furthermore, while modulo by most negative constant happens to return the
right value, it does that only by invoking UB in the IL, because
we then expand division by that 1U+INT_MAX and say for INT_MIN % INT_MIN
compute the division as -1, and then multiply by INT_MIN, which is signed
integer overflow.  We in theory could do the computation in unsigned vector
types instead, but is it worth bothering.  People that are doing % INT_MIN
are either testing for standard conformance, or doing something wrong.
So, I've also added punting on % INT_MIN, both in vect lowering and vect
pattern recognition (we punt already for / INT_MIN).

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

2020-04-08  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/94524
	* tree-vect-generic.c (expand_vector_divmod): If any elt of op1 is
	negative for signed TRUNC_MOD_EXPR, multiply with absolute value of
	op1 rather than op1 itself at the end.  Punt for signed modulo by
	most negative constant.
	* tree-vect-patterns.c (vect_recog_divmod_pattern): Punt for signed
	modulo by most negative constant.

	* gcc.c-torture/execute/pr94524-1.c: New test.
	* gcc.c-torture/execute/pr94524-2.c: New test.


	Jakub

On April 8, 2020 8:14:56 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>The first testcase below is miscompiled, because for the division part
>of the lowering we canonicalize negative divisors to their absolute
>value
>(similarly how expmed.c canonicalizes it), but when multiplying the
>division
>result back by the VECTOR_CST, we use the original constant, which can
>contain negative divisors.
>
>Fixed by computing ABS_EXPR of the VECTOR_CST.  Unfortunately,
>fold-const.c
>doesn't support const_unop (ABS_EXPR, VECTOR_CST) and I think it is too
>late
>in GCC 10 cycle to add it now.
>
>Furthermore, while modulo by most negative constant happens to return
>the
>right value, it does that only by invoking UB in the IL, because
>we then expand division by that 1U+INT_MAX and say for INT_MIN %
>INT_MIN
>compute the division as -1, and then multiply by INT_MIN, which is
>signed
>integer overflow.  We in theory could do the computation in unsigned
>vector
>types instead, but is it worth bothering.  People that are doing %
>INT_MIN
>are either testing for standard conformance, or doing something wrong.
>So, I've also added punting on % INT_MIN, both in vect lowering and
>vect
>pattern recognition (we punt already for / INT_MIN).
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK. 

Richard 

>2020-04-08  Jakub Jelinek  <jakub@redhat.com>
>
>	PR tree-optimization/94524
>	* tree-vect-generic.c (expand_vector_divmod): If any elt of op1 is
>	negative for signed TRUNC_MOD_EXPR, multiply with absolute value of
>	op1 rather than op1 itself at the end.  Punt for signed modulo by
>	most negative constant.
>	* tree-vect-patterns.c (vect_recog_divmod_pattern): Punt for signed
>	modulo by most negative constant.
>
>	* gcc.c-torture/execute/pr94524-1.c: New test.
>	* gcc.c-torture/execute/pr94524-2.c: New test.
>
>--- gcc/tree-vect-generic.c.jj	2020-01-12 11:54:38.518381650 +0100
>+++ gcc/tree-vect-generic.c	2020-04-08 11:46:34.411988882 +0200
>@@ -478,6 +478,7 @@ expand_vector_divmod (gimple_stmt_iterat
> {
>   bool use_pow2 = true;
>   bool has_vector_shift = true;
>+  bool use_abs_op1 = false;
>   int mode = -1, this_mode;
>   int pre_shift = -1, post_shift;
>   unsigned int nunits = nunits_for_known_piecewise_op (type);
>@@ -618,8 +619,11 @@ expand_vector_divmod (gimple_stmt_iterat
> 
> 	  /* n rem d = n rem -d */
> 	  if (code == TRUNC_MOD_EXPR && d < 0)
>-	    d = abs_d;
>-	  else if (abs_d == HOST_WIDE_INT_1U << (prec - 1))
>+	    {
>+	      d = abs_d;
>+	      use_abs_op1 = true;
>+	    }
>+	  if (abs_d == HOST_WIDE_INT_1U << (prec - 1))
> 	    {
> 	      /* This case is not handled correctly below.  */
> 	      mode = -2;
>@@ -899,6 +903,23 @@ expand_vector_divmod (gimple_stmt_iterat
>   if (op == unknown_optab
>       || optab_handler (op, TYPE_MODE (type)) == CODE_FOR_nothing)
>     return NULL_TREE;
>+  if (use_abs_op1)
>+    {
>+      tree_vector_builder elts;
>+      if (!elts.new_unary_operation (type, op1, false))
>+	return NULL_TREE;
>+      unsigned int count = elts.encoded_nelts ();
>+      for (unsigned int i = 0; i < count; ++i)
>+	{
>+	  tree elem1 = VECTOR_CST_ELT (op1, i);
>+
>+	  tree elt = const_unop (ABS_EXPR, TREE_TYPE (elem1), elem1);
>+	  if (elt == NULL_TREE)
>+	    return NULL_TREE;
>+	  elts.quick_push (elt);
>+	}
>+      op1 = elts.build ();
>+    }
>   tem = gimplify_build2 (gsi, MULT_EXPR, type, cur_op, op1);
>   op = optab_for_tree_code (MINUS_EXPR, type, optab_default);
>   if (op == unknown_optab
>--- gcc/tree-vect-patterns.c.jj	2020-01-12 11:54:38.520381620 +0100
>+++ gcc/tree-vect-patterns.c	2020-04-08 11:47:05.540523021 +0200
>@@ -3365,8 +3365,8 @@ vect_recog_divmod_pattern (stmt_vec_info
> 	  d = abs_d;
> 	  oprnd1 = build_int_cst (itype, abs_d);
> 	}
>-      else if (HOST_BITS_PER_WIDE_INT >= prec
>-	       && abs_d == HOST_WIDE_INT_1U << (prec - 1))
>+      if (HOST_BITS_PER_WIDE_INT >= prec
>+	  && abs_d == HOST_WIDE_INT_1U << (prec - 1))
> 	/* This case is not handled correctly below.  */
> 	return NULL;
> 
>--- gcc/testsuite/gcc.c-torture/execute/pr94524-1.c.jj	2020-04-08
>11:48:37.357148916 +0200
>+++ gcc/testsuite/gcc.c-torture/execute/pr94524-1.c	2020-04-08
>11:50:01.152894857 +0200
>@@ -0,0 +1,19 @@
>+/* PR tree-optimization/94524 */
>+
>+typedef signed char __attribute__ ((__vector_size__ (16))) V;
>+
>+static __attribute__ ((__noinline__, __noclone__)) V
>+foo (V c)
>+{
>+  c %= (signed char) -19;
>+  return (V) c;
>+}
>+
>+int
>+main ()
>+{
>+  V x = foo ((V) { 31 });
>+  if (x[0] != 12)
>+    __builtin_abort ();
>+  return 0;
>+}
>--- gcc/testsuite/gcc.c-torture/execute/pr94524-2.c.jj	2020-04-08
>11:48:40.694098980 +0200
>+++ gcc/testsuite/gcc.c-torture/execute/pr94524-2.c	2020-04-08
>11:50:13.049716806 +0200
>@@ -0,0 +1,25 @@
>+/* PR tree-optimization/94524 */
>+
>+typedef signed char __attribute__ ((__vector_size__ (16))) V;
>+
>+static __attribute__ ((__noinline__, __noclone__)) V
>+foo (V c)
>+{
>+  c %= (signed char) -128;
>+  return (V) c;
>+}
>+
>+int
>+main ()
>+{
>+  V x = foo ((V) { -128 });
>+  if (x[0] != 0)
>+    __builtin_abort ();
>+  x = foo ((V) { -127 });
>+  if (x[0] != -127)
>+    __builtin_abort ();
>+  x = foo ((V) { 127 });
>+  if (x[0] != 127)
>+    __builtin_abort ();
>+  return 0;
>+}
>
>	Jakub