public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Michael Meissner <meissner@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc(refs/users/meissner/heads/ieee)] vect: Fix up lowering of TRUNC_MOD_EXPR by negative constant [PR94524]
Date: Thu, 16 Apr 2020 23:00:41 +0000 (GMT)	[thread overview]
Message-ID: <20200416230041.B3A98384B400@sourceware.org> (raw)

https://gcc.gnu.org/g:f52eb4f988992d393c69ee4ab76f236dced80e36

commit f52eb4f988992d393c69ee4ab76f236dced80e36
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 8 21:22:05 2020 +0200

    vect: Fix up lowering of TRUNC_MOD_EXPR by negative constant [PR94524]
    
    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).
    
    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.

Diff:
---
 gcc/ChangeLog                                   | 10 ++++++++++
 gcc/testsuite/ChangeLog                         |  4 ++++
 gcc/testsuite/gcc.c-torture/execute/pr94524-1.c | 19 +++++++++++++++++++
 gcc/testsuite/gcc.c-torture/execute/pr94524-2.c | 25 +++++++++++++++++++++++++
 gcc/tree-vect-generic.c                         | 25 +++++++++++++++++++++++--
 gcc/tree-vect-patterns.c                        |  4 ++--
 6 files changed, 83 insertions(+), 4 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 025a4a67937..dcdfae1ae8c 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@
+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.
+
 2020-04-08  Richard Biener  <rguenther@suse.de>
 
 	PR rtl-optimization/93946
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 8916b897cc4..134280d1369 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,5 +1,9 @@
 2020-04-08  Jakub Jelinek  <jakub@redhat.com>
 
+	PR tree-optimization/94524
+	* gcc.c-torture/execute/pr94524-1.c: New test.
+	* gcc.c-torture/execute/pr94524-2.c: New test.
+
 	PR c++/94314
 	* g++.dg/pr94314.C (A::operator new, B::operator new, C::operator new):
 	Use __SIZE_TYPE__ instead of unsigned long.
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94524-1.c b/gcc/testsuite/gcc.c-torture/execute/pr94524-1.c
new file mode 100644
index 00000000000..e7365ad97fd
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr94524-1.c
@@ -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;
+}
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94524-2.c b/gcc/testsuite/gcc.c-torture/execute/pr94524-2.c
new file mode 100644
index 00000000000..9c74b7be403
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr94524-2.c
@@ -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;
+}
diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c
index 2f6fd5e980c..8b00f325054 100644
--- a/gcc/tree-vect-generic.c
+++ b/gcc/tree-vect-generic.c
@@ -478,6 +478,7 @@ expand_vector_divmod (gimple_stmt_iterator *gsi, tree type, tree op0,
 {
   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_iterator *gsi, tree type, tree op0,
 
 	  /* 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_iterator *gsi, tree type, tree op0,
   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
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 60dc1bfeaa4..dd0c19dc5e5 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -3365,8 +3365,8 @@ vect_recog_divmod_pattern (stmt_vec_info stmt_vinfo, tree *type_out)
 	  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;


                 reply	other threads:[~2020-04-16 23:00 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200416230041.B3A98384B400@sourceware.org \
    --to=meissner@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).