On Wed, 14 Oct 2015, Marek Polacek wrote:

Evidently, the X - (X / Y) * Y -> X % Y pattern can't change the
signedness of X from signed to unsigned, otherwise we'd generate
wrong code.  (But unsigned -> signed should be fine.)

Does anyone see a better fix than this?

I was wondering about producing:

(convert (trunc_mod @0 @1))

but didn't really think about it.

Aren't there also problems if the conversion changes the precision? I can imagine your patch ending in x % 0, with @1 non-zero but a narrowing cast.

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

2015-10-14  Marek Polacek  <pola...@redhat.com>

        PR tree-optimization/67953
        * match.pd (X - (X / Y) * Y): Don't change signedness of @0.

        * gcc.dg/fold-minus-6.c (fn4): Change the type of A to
        unsigned.
        * gcc.dg/torture/pr67953.c: New test.

diff --git gcc/match.pd gcc/match.pd
index 655c9ff..24e19a9 100644
--- gcc/match.pd
+++ gcc/match.pd
@@ -267,7 +267,8 @@ along with GCC; see the file COPYING3.  If not see
/* X - (X / Y) * Y is the same as X % Y.  */
(simplify
 (minus (convert1? @0) (convert2? (mult (trunc_div @0 @1) @1)))
- (if (INTEGRAL_TYPE_P (type) || VECTOR_INTEGER_TYPE_P (type))
+ (if ((INTEGRAL_TYPE_P (type) || VECTOR_INTEGER_TYPE_P (type))
+      && TYPE_UNSIGNED (TREE_TYPE (@0)) == TYPE_UNSIGNED (type))
  (trunc_mod (convert @0) (convert @1))))

/* Optimize TRUNC_MOD_EXPR by a power of two into a BIT_AND_EXPR,
diff --git gcc/testsuite/gcc.dg/fold-minus-6.c 
gcc/testsuite/gcc.dg/fold-minus-6.c
index 1c22c25..1535452 100644
--- gcc/testsuite/gcc.dg/fold-minus-6.c
+++ gcc/testsuite/gcc.dg/fold-minus-6.c
@@ -20,7 +20,7 @@ fn3 (long int x)
}

int
-fn4 (int a, int b)
+fn4 (unsigned int a, int b)
{
  return a - (unsigned) ((a / b) * b);
}
diff --git gcc/testsuite/gcc.dg/torture/pr67953.c 
gcc/testsuite/gcc.dg/torture/pr67953.c
index e69de29..5ce399b 100644
--- gcc/testsuite/gcc.dg/torture/pr67953.c
+++ gcc/testsuite/gcc.dg/torture/pr67953.c
@@ -0,0 +1,42 @@
+/* PR tree-optimization/67953 */
+/* { dg-do run } */
+
+unsigned int
+fn1 (signed int a)
+{
+  return (unsigned int) a - ((a / 3) * 3);
+}
+
+unsigned int
+fn2 (signed int a)
+{
+  return a - ((a / 3) * 3);
+}
+
+unsigned int
+fn3 (int a)
+{
+  return a - (unsigned) ((a / 3) * 3);
+}
+
+signed int
+fn4 (int a)
+{
+  return (unsigned) a - (unsigned) ((a / 3) * 3);
+}
+
+signed int
+fn5 (unsigned int a)
+{
+  return (signed) a - (int) ((a / 3) * 3);
+}
+
+int
+main ()
+{
+  if (fn1 (-5) != -2
+      || fn2 (-5) != -2
+      || fn3 (-5) != -2
+      || fn4 (-5) != -2)
+    __builtin_abort ();
+}

        Marek


--
Marc Glisse

Reply via email to