On April 8, 2020 8:14:56 PM GMT+02:00, Jakub Jelinek <ja...@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 <ja...@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
