[PATCH] Fix typo, fixing PR87465

[PATCH] Fix typo, fixing PR87465

[Richard Biener]

The following typo-fix happens to fix a --param max-peel-branches limit
caused missed peeling.  The typo is present everywhere, the missed
peeling is a regression from GCC 7.

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

I'm not really considering to backport this anywhere.  Note the
testcase isn't fully optimized on the tree level because
DOM doesn't figure out the trivial CSE after SLP vectorizes the
array init (we have PRs for that issue).

Richard.

2018-10-01  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/87465
	* tree-ssa-loop-ivcanon.c (tree_estimate_loop_size): Fix typo
	causing branch miscounts.

	* gcc.dg/tree-ssa/cunroll-15.c: New testcase.

Index: gcc/tree-ssa-loop-ivcanon.c
===================================================================
--- gcc/tree-ssa-loop-ivcanon.c	(revision 264734)
+++ gcc/tree-ssa-loop-ivcanon.c	(working copy)
@@ -368,8 +368,8 @@ tree_estimate_loop_size (struct loop *lo
 	    size->non_call_stmts_on_hot_path++;
 	  if (((gimple_code (stmt) == GIMPLE_COND
 	        && (!constant_after_peeling (gimple_cond_lhs (stmt), stmt, loop)
-		    || constant_after_peeling (gimple_cond_rhs (stmt), stmt,
-					       loop)))
+		    || !constant_after_peeling (gimple_cond_rhs (stmt), stmt,
+						loop)))
 	       || (gimple_code (stmt) == GIMPLE_SWITCH
 		   && !constant_after_peeling (gimple_switch_index (
 						 as_a <gswitch *> (stmt)),
Index: gcc/testsuite/gcc.dg/tree-ssa/cunroll-15.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/cunroll-15.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/cunroll-15.c	(working copy)
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fdump-tree-cunroll-optimized" } */
+
+int Test(void)
+{
+  int c = 0;
+  const int in[4] = {4,3,4,4};
+  for (unsigned i = 0; i < 4; i++) {
+      for (unsigned j = 0; j < i; j++) {
+	  if (in[i] == in[j])
+	    break;
+	  else 
+	    ++c;
+      }
+  }
+  return c;
+}
+
+/* { dg-final { scan-tree-dump-times "optimized:\[^\n\r\]*completely unrolled" 2 "cunroll" } } */
+/* Only RTL figures out some CSE at the moment.  */
+/* { dg-final { scan-tree-dump "return 1;" "optimized" { xfail *-*-* } } } */

Re: [PATCH] Fix typo, fixing PR87465

[Christophe Lyon]

On Mon, 1 Oct 2018 at 11:36, Richard Biener <rguenther@suse.de> wrote:
>
>
> The following typo-fix happens to fix a --param max-peel-branches limit
> caused missed peeling.  The typo is present everywhere, the missed
> peeling is a regression from GCC 7.
>
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
>
> I'm not really considering to backport this anywhere.  Note the
> testcase isn't fully optimized on the tree level because
> DOM doesn't figure out the trivial CSE after SLP vectorizes the
> array init (we have PRs for that issue).
>
> Richard.
>
> 2018-10-01  Richard Biener  <rguenther@suse.de>
>
>         PR tree-optimization/87465
>         * tree-ssa-loop-ivcanon.c (tree_estimate_loop_size): Fix typo
>         causing branch miscounts.
>
>         * gcc.dg/tree-ssa/cunroll-15.c: New testcase.
>

Hi,

The new testcase fails on arm and powerpc.


> Index: gcc/tree-ssa-loop-ivcanon.c
> ===================================================================
> --- gcc/tree-ssa-loop-ivcanon.c (revision 264734)
> +++ gcc/tree-ssa-loop-ivcanon.c (working copy)
> @@ -368,8 +368,8 @@ tree_estimate_loop_size (struct loop *lo
>             size->non_call_stmts_on_hot_path++;
>           if (((gimple_code (stmt) == GIMPLE_COND
>                 && (!constant_after_peeling (gimple_cond_lhs (stmt), stmt, loop)
> -                   || constant_after_peeling (gimple_cond_rhs (stmt), stmt,
> -                                              loop)))
> +                   || !constant_after_peeling (gimple_cond_rhs (stmt), stmt,
> +                                               loop)))
>                || (gimple_code (stmt) == GIMPLE_SWITCH
>                    && !constant_after_peeling (gimple_switch_index (
>                                                  as_a <gswitch *> (stmt)),
> Index: gcc/testsuite/gcc.dg/tree-ssa/cunroll-15.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/cunroll-15.c  (nonexistent)
> +++ gcc/testsuite/gcc.dg/tree-ssa/cunroll-15.c  (working copy)
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fdump-tree-cunroll-optimized" } */
> +
> +int Test(void)
> +{
> +  int c = 0;
> +  const int in[4] = {4,3,4,4};
> +  for (unsigned i = 0; i < 4; i++) {
> +      for (unsigned j = 0; j < i; j++) {
> +         if (in[i] == in[j])
> +           break;
> +         else
> +           ++c;
> +      }
> +  }
> +  return c;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "optimized:\[^\n\r\]*completely unrolled" 2 "cunroll" } } */
> +/* Only RTL figures out some CSE at the moment.  */
> +/* { dg-final { scan-tree-dump "return 1;" "optimized" { xfail *-*-* } } } */

Re: [PATCH] Fix typo, fixing PR87465

[Richard Biener]

On Wed, 3 Oct 2018, Christophe Lyon wrote:

> On Mon, 1 Oct 2018 at 11:36, Richard Biener <rguenther@suse.de> wrote:
> >
> >
> > The following typo-fix happens to fix a --param max-peel-branches limit
> > caused missed peeling.  The typo is present everywhere, the missed
> > peeling is a regression from GCC 7.
> >
> > Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> >
> > I'm not really considering to backport this anywhere.  Note the
> > testcase isn't fully optimized on the tree level because
> > DOM doesn't figure out the trivial CSE after SLP vectorizes the
> > array init (we have PRs for that issue).
> >
> > Richard.
> >
> > 2018-10-01  Richard Biener  <rguenther@suse.de>
> >
> >         PR tree-optimization/87465
> >         * tree-ssa-loop-ivcanon.c (tree_estimate_loop_size): Fix typo
> >         causing branch miscounts.
> >
> >         * gcc.dg/tree-ssa/cunroll-15.c: New testcase.
> >
> 
> Hi,
> 
> The new testcase fails on arm and powerpc.

Bah, it's the old issue that DOM doesn't handle CSE of

  in = *.LC0;
  _52 = in[1];
  _51 = in[0];

I see no other way of xfailing for a specific list of targets then.

Please feel free to amend if new ones pop up.

I'm committing the following.  I really wonder if we could go
for a cheap non-iterating FRE now (and whether late jump threading
from DOM is important).

Richard.

2018-10-04  Richard Biener  <rguenther@suse.de>

	* gcc.dg/tree-ssa/cunroll-15.c: Add XFAILs for arm and powerpc.

Index: gcc/testsuite/gcc.dg/tree-ssa/cunroll-15.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/cunroll-15.c	(revision 264835)
+++ gcc/testsuite/gcc.dg/tree-ssa/cunroll-15.c	(working copy)
@@ -19,4 +19,5 @@ int Test(void)
 /* { dg-final { scan-tree-dump-times "optimized:\[^\n\r\]*completely unrolled" 2 "cunroll" } } */
 /* When SLP vectorization is enabled the following will fail because DOM
    doesn't know how to deal with the vectorized initializer of in.  */
-/* { dg-final { scan-tree-dump "return 1;" "optimized" } } */
+/* DOM also doesn't know to CSE in[1] with in = *.LC0 thus the list of targets this fails.  */
+/* { dg-final { scan-tree-dump "return 1;" "optimized" { xfail { arm*-*-* powerpc-*-* } } } } */

Re: [PATCH] Fix typo, fixing PR87465

[Jeff Law]

On 10/4/18 2:58 AM, Richard Biener wrote:
> On Wed, 3 Oct 2018, Christophe Lyon wrote:
> 
>> On Mon, 1 Oct 2018 at 11:36, Richard Biener <rguenther@suse.de> wrote:
>>>
>>>
>>> The following typo-fix happens to fix a --param max-peel-branches limit
>>> caused missed peeling.  The typo is present everywhere, the missed
>>> peeling is a regression from GCC 7.
>>>
>>> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
>>>
>>> I'm not really considering to backport this anywhere.  Note the
>>> testcase isn't fully optimized on the tree level because
>>> DOM doesn't figure out the trivial CSE after SLP vectorizes the
>>> array init (we have PRs for that issue).
>>>
>>> Richard.
>>>
>>> 2018-10-01  Richard Biener  <rguenther@suse.de>
>>>
>>>         PR tree-optimization/87465
>>>         * tree-ssa-loop-ivcanon.c (tree_estimate_loop_size): Fix typo
>>>         causing branch miscounts.
>>>
>>>         * gcc.dg/tree-ssa/cunroll-15.c: New testcase.
>>>
>>
>> Hi,
>>
>> The new testcase fails on arm and powerpc.
> 
> Bah, it's the old issue that DOM doesn't handle CSE of
> 
>   in = *.LC0;
>   _52 = in[1];
>   _51 = in[0];
Right.  It doesn't work hard at all to hash alternate forms of memory
references to the same hash value.  It's one of the reasons I'd like to
move away from its hash routines to a standardized VN like you've
implemented elsewhere as I believe your VN code does a better job at
finding these alternate forms and generating a consistent VN for them.


> 
> I see no other way of xfailing for a specific list of targets then.
> 
> Please feel free to amend if new ones pop up.
> 
> I'm committing the following.  I really wonder if we could go
> for a cheap non-iterating FRE now (and whether late jump threading
> from DOM is important).
It's not hugely important, though with the desire to kill the VRP
threading, the late DOM threading increases slightly in importance.

jeff