diff mbox

Fix PR31096

Message ID CO2PR07MB2694EDC68D383E78CA227D768E900@CO2PR07MB2694.namprd07.prod.outlook.com
State New
Headers show

Commit Message

Hurugalawadi, Naveen April 7, 2016, 11:04 a.m. UTC 
Hi,

Thanks for the review, views and comments on the issue.

>> -1 is an integer constant, so that's still invalid. It is also invalid for
>> unsigned. The :s are useless since the output is a single insn.

The patch is modified as per your review comments.

Currently the following conditions had been taken care in the patch
as per your suggestions:-

a * c op b * c -> Optimizes for all cases
a * c op b * c -> Does not optimize when  c = 0

a * -c eq/ne b * -c -> Optimizes for all cases
a * -c lt/gt/ge/le b * -c -> Optimizes when c is positive
a * -c lt/gt/ge/le b * -c -> Optimizes and becomes b lt/gt/ge/le when c is negative

Have added a minimal testcase which covers all the above instances.
Please review the patch and let me know if its okay?

Thanks,
Naveen

Comments

Marc Glisse April 7, 2016, 11:28 a.m. UTC | #1 
On Thu, 7 Apr 2016, Hurugalawadi, Naveen wrote:

> +/* Fold A * 10 == B * 10 into A == B.  */
> +(for cmp (eq ne)
> + (simplify
> +  (cmp (mult:c @0 @1) (mult:c @2 @1))
> +  (if (TYPE_OVERFLOW_UNDEFINED (type)

type is the return type of the comparison. The relevant type here is
TREE_TYPE (@0). Maybe add a testcase with unsigned, to check that it
does not transform?

> +       && !integer_zerop (@1))
> +   (cmp @0 @2))))

!integer_zerop is not a promise that the variable is not zero, it just
says that we don't know for sure that it is zero. integer_nonzerop would
work. Or writing (cmp (mult @0 INTEGER_CST@1) (mult @2 @1)), but
then !wi::eq_p (@1, 0) would be a better test.

To make it more general (not limited to constants), you could probably
use tree_expr_nonzero_p, and at some point someone would enhance
tree_single_nonzero_warnv_p by checking VRP information for SSA_NAME.

The other transformation has similar issues.
diff mbox

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index 75aa601..9386172 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -894,7 +894,24 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
       && tree_nop_conversion_p (type, TREE_TYPE (@1)))
   (convert (bit_and (bit_not @1) @0))))
 
+/* Fold A * 10 == B * 10 into A == B.  */
+(for cmp (eq ne)
+ (simplify
+  (cmp (mult:c @0 @1) (mult:c @2 @1))
+  (if (TYPE_OVERFLOW_UNDEFINED (type)
+       && !integer_zerop (@1))
+   (cmp @0 @2))))
 
+/* Fold A * 10 < B * 10 into A < B.  */
+(for cmp (lt gt le ge)
+ (simplify
+  (cmp (mult:c @0 @1) (mult:c @2 @1))
+  (if (TYPE_OVERFLOW_UNDEFINED (type)
+       && !integer_zerop (@1))
+   (if (tree_expr_nonnegative_p (@1))
+    (cmp @0 @2))
+   (if (!tree_expr_nonnegative_p (@1))
+    (cmp @2 @0)))))
 
 /* ((X inner_op C0) outer_op C1)
    With X being a tree where value_range has reasoned certain bits to always be
diff --git a/gcc/testsuite/gcc.dg/pr31096-1.c b/gcc/testsuite/gcc.dg/pr31096-1.c
new file mode 100644
index 0000000..8489724
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr31096-1.c
@@ -0,0 +1,29 @@ 
+/* PR middle-end/31096 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" }  */
+
+int
+f (int a, int b)
+{
+  return a > b;
+}
+
+int
+f1 (int a, int b)
+{
+  return a * 10 >= b * 10;
+}
+
+int
+f2 (int a, int b)
+{
+  return a * -42 <  b * -42;
+}
+
+int
+f3 (int a, int b)
+{
+  return a * 0 <= b * 0;
+}
+
+/* { dg-final { scan-tree-dump-not "\(D\) * " "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/pr31096.c b/gcc/testsuite/gcc.dg/pr31096.c
new file mode 100644
index 0000000..05536ad
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr31096.c
@@ -0,0 +1,29 @@ 
+/* PR middle-end/31096 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" }  */
+
+int
+f (int a, int b)
+{
+  return a == b;
+}
+
+int
+f1 (int a, int b)
+{
+  return a * 10 == b * 10;
+}
+
+int
+f2 (int a, int b)
+{
+  return a * -42 !=  b * -42;
+}
+
+int
+f3 (int a, int b)
+{
+  return a * 0 != b * 0;
+}
+
+/* { dg-final { scan-tree-dump-not "\(D\) * " "optimized" } } */