[ PATCH ] Split out my fold-const.c change and add a test case

[ PATCH ] Split out my fold-const.c change and add a test case

[Robert Kennedy]

This is my recent fold-const change (from
http://gcc.gnu.org/ml/gcc-patches/2007-01/msg00108.html) to which
Richard Guenther said, "The fold-const change is ok if you add a
testcase." So here is the change with a testcase.

	-- Robert

		    ------------------------------

2007-01-04  Robert Kennedy <jimbob@google.com>
	* fold-const.c (fold_comparison): Fold comparisons like (x *
	1000 < 0) to (x < 0).
	* fold-compare-2.c: New test case for fold_comparison.
	
==== //depot2/gcctools/google_vendor_src_branch/gcc/trunk/gcc/fold-const.c#18 - /home/jimbob/clients/jimbob-perforce2-test/gcctools/google_vendor_src_branch/gcc/trunk/gcc/fold-const.c ====
# action=edit type=text
--- gcctools/google_vendor_src_branch/gcc/trunk/gcc/fold-const.c	2007-01-04 18:16:49.000000000 -0800
+++ gcctools/google_vendor_src_branch/gcc/trunk/gcc/fold-const.c	2007-01-04 18:14:25.000000000 -0800
@@ -8169,6 +8169,29 @@
 			    variable2);
     }
 
+  /* Transform comparisons of the form X * C1 CMP 0 to X CMP 0 in the
+     signed arithmetic case.  That form is created by the compiler
+     often enough for folding it to be of value.  One example is in
+     computing loop trip counts after Operator Strength Reduction.  */
+  if (!(flag_wrapv || flag_trapv)
+      && !TYPE_UNSIGNED (TREE_TYPE (arg0))
+      && TREE_CODE (arg0) == MULT_EXPR
+      && (TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
+          && !TREE_OVERFLOW (TREE_OPERAND (arg0, 1)))
+      && integer_zerop (arg1))
+    {
+      tree const1 = TREE_OPERAND (arg0, 1);
+      tree const2 = arg1;                       /* zero */
+      tree variable1 = TREE_OPERAND (arg0, 0);
+      enum tree_code cmp_code = code;
+
+      /* If const1 is negative we swap the sense of the comparison.  */
+      if (tree_int_cst_sgn (const1) < 0)
+        cmp_code = swap_tree_comparison (cmp_code);
+
+      return fold_build2 (cmp_code, type, variable1, const2);
+    }
+
   tem = maybe_canonicalize_comparison (code, type, arg0, arg1);
   if (tem)
     return tem;
==== //depot2/gcctools/google_vendor_src_branch/gcc/trunk/gcc/testsuite/gcc.dg/fold-compare-2.c#1 - /home/jimbob/clients/jimbob-perforce2-test/gcctools/google_vendor_src_branch/gcc/trunk/gcc/testsuite/gcc.dg/fold-compare-2.c ====
# action=add type=text
--- /dev/null	1969-12-31 16:00:00.000000000 -0800
+++ gcctools/google_vendor_src_branch/gcc/trunk/gcc/testsuite/gcc.dg/fold-compare-2.c	2007-01-04 16:56:33.000000000 -0800
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-vrp" } */
+
+extern void abort (void);
+
+int a;
+
+int
+main(void)
+{
+  if (a * 1000 < 0)
+    abort ();
+  if (a * -43 > 0)
+    abort ();
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } */

Re: [ PATCH ] Split out my fold-const.c change and add a test case

[Mark Mitchell]

Robert Kennedy wrote:

> 2007-01-04  Robert Kennedy <jimbob@google.com>
> 	* fold-const.c (fold_comparison): Fold comparisons like (x *
> 	1000 < 0) to (x < 0).
> 	* fold-compare-2.c: New test case for fold_comparison.

> +  /* Transform comparisons of the form X * C1 CMP 0 to X CMP 0 in the
> +     signed arithmetic case.  That form is created by the compiler
> +     often enough for folding it to be of value.  One example is in
> +     computing loop trip counts after Operator Strength Reduction.  */
> +  if (!(flag_wrapv || flag_trapv)
> +      && !TYPE_UNSIGNED (TREE_TYPE (arg0))
> +      && TREE_CODE (arg0) == MULT_EXPR
> +      && (TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
> +          && !TREE_OVERFLOW (TREE_OPERAND (arg0, 1)))
> +      && integer_zerop (arg1))
> +    {
> +      tree const1 = TREE_OPERAND (arg0, 1);

Would you please add:

  gcc_assert (!integer_zerop (const1))

?

I'm assuming that you know at this point const1 can never be zero,
because of something somewhere else in this function or in the compiler,
as otherwise the optimization would of course be invalid.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [ PATCH ] Split out my fold-const.c change and add a test case

[Richard Guenther]

On 1/5/07, Mark Mitchell <mark@codesourcery.com> wrote:
> Robert Kennedy wrote:
>
> > 2007-01-04  Robert Kennedy <jimbob@google.com>
> >       * fold-const.c (fold_comparison): Fold comparisons like (x *
> >       1000 < 0) to (x < 0).
> >       * fold-compare-2.c: New test case for fold_comparison.
>
> > +  /* Transform comparisons of the form X * C1 CMP 0 to X CMP 0 in the
> > +     signed arithmetic case.  That form is created by the compiler
> > +     often enough for folding it to be of value.  One example is in
> > +     computing loop trip counts after Operator Strength Reduction.  */
> > +  if (!(flag_wrapv || flag_trapv)
> > +      && !TYPE_UNSIGNED (TREE_TYPE (arg0))
> > +      && TREE_CODE (arg0) == MULT_EXPR
> > +      && (TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST
> > +          && !TREE_OVERFLOW (TREE_OPERAND (arg0, 1)))
> > +      && integer_zerop (arg1))
> > +    {
> > +      tree const1 = TREE_OPERAND (arg0, 1);
>
> Would you please add:
>
>   gcc_assert (!integer_zerop (const1))
>
> ?
>
> I'm assuming that you know at this point const1 can never be zero,
> because of something somewhere else in this function or in the compiler,
> as otherwise the optimization would of course be invalid.

Indeed.  But if this ever happened we'd have an unfolded tree which
would be another bug.  Of course silent wrong-code in this case is worse
than an ICE, so I agree with the assert.

Richard.

Re: [ PATCH ] Split out my fold-const.c change and add a test case

[Robert Kennedy]

> Indeed.  But if this ever happened we'd have an unfolded tree which
> would be another bug.  Of course silent wrong-code in this case is worse
> than an ICE, so I agree with the assert.

Ping.

Perhaps I should have followed up in this thread instead, but
yesterday evening I posted:
  http://gcc.gnu.org/ml/gcc-patches/2007-01/msg00634.html

Is that patch OK now? The assert is added and the test case is there.

Thanks!

	-- Robert

Re: [ PATCH ] Split out my fold-const.c change and add a test case

[Richard Guenther]

On 1/10/07, Mark Mitchell <mark@codesourcery.com> wrote:
> Robert Kennedy wrote:
> >> Indeed.  But if this ever happened we'd have an unfolded tree which
> >> would be another bug.  Of course silent wrong-code in this case is worse
> >> than an ICE, so I agree with the assert.
> >
> > Ping.
> >
> > Perhaps I should have followed up in this thread instead, but
> > yesterday evening I posted:
> >   http://gcc.gnu.org/ml/gcc-patches/2007-01/msg00634.html
> >
> > Is that patch OK now? The assert is added and the test case is there.
>
> It's fine by me.  If nobody else comments in 24 hours, check it in.
> But, I want to give Richard G. a chance to comment, if he has any concerns.

The patch is ok.

Thanks,
Richard.