Re: [PATCH] Fix PR72772

[Bin.Cheng]

On Fri, Aug 5, 2016 at 11:48 AM, Richard Biener  wrote:
>
> This fixes PR72772 by avoing placing a degenerate PHI in each
> forwarder block loop init creates when creating simple preheaders.
> The solution is to simply split the single loop entry edge which
> is also way cheaper than using the forwarder block creation path.
>
> You've seen a load of fallout fixes already, so this is the final
> patch adjusting two testcases (for gcc.dg/tree-ssa/pr59597.c we
> no longer register the unwanted threadings as the forwarders
> no longer contain PHIs).
>
> This patch will cause
>
> +FAIL: gcc.dg/graphite/scop-dsyr2k.c scan-tree-dump-times graphite "number
> of SCoPs: 1" 1
> +FAIL: gcc.dg/graphite/scop-dsyrk.c scan-tree-dump-times graphite "number
> of SCoPs: 1" 1
>
> on x86_64 with -m32 as niter analysis is confused by us now generating
> an optimized loop nest via threading that has all redundant checks
> removed.  We no longer can prove that the loops do not eventually
> iterate zero times.  I will open a PR for this (it is a latent issue).
> The tests would pass with VRP disabled but I choose to leave them
> FAILing for now.
>
> Bootstrapped / tested many times on x86_64-unknown-linux-gnu, re-doing
> this after the latest fallout fix now.
>
> Richard.
>
> 2016-08-05  Richard Biener  
>
> PR tree-optimization/72772
> * cfgloopmanip.c (create_preheader): Use split_edge if there
> is a single loop entry, avoiding degenerate PHIs.
>
> * gcc.dg/tree-ssa/ldist-24.c: New testcase.
FYI, I committed the same test as gcc.dg/tree-ssa/pr72772.c.  If it's
appropriate, this ldist test can be saved here.

Thanks,
bin

[PATCH] Fix PR72772

[Richard Biener]

This fixes PR72772 by avoing placing a degenerate PHI in each
forwarder block loop init creates when creating simple preheaders.
The solution is to simply split the single loop entry edge which
is also way cheaper than using the forwarder block creation path.

You've seen a load of fallout fixes already, so this is the final
patch adjusting two testcases (for gcc.dg/tree-ssa/pr59597.c we
no longer register the unwanted threadings as the forwarders
no longer contain PHIs).

This patch will cause

+FAIL: gcc.dg/graphite/scop-dsyr2k.c scan-tree-dump-times graphite "number 
of SCoPs: 1" 1
+FAIL: gcc.dg/graphite/scop-dsyrk.c scan-tree-dump-times graphite "number 
of SCoPs: 1" 1

on x86_64 with -m32 as niter analysis is confused by us now generating
an optimized loop nest via threading that has all redundant checks 
removed.  We no longer can prove that the loops do not eventually
iterate zero times.  I will open a PR for this (it is a latent issue).
The tests would pass with VRP disabled but I choose to leave them
FAILing for now.

Bootstrapped / tested many times on x86_64-unknown-linux-gnu, re-doing
this after the latest fallout fix now.

Richard.

2016-08-05  Richard Biener  

PR tree-optimization/72772
* cfgloopmanip.c (create_preheader): Use split_edge if there
is a single loop entry, avoiding degenerate PHIs.

* gcc.dg/tree-ssa/ldist-24.c: New testcase.
* gcc.dg/graphite/pr35356-1.c: Adjust.
* gcc.dg/tree-ssa/pr59597.c: Likewise.

Index: gcc/cfgloopmanip.c
===
*** gcc/cfgloopmanip.c  (revision 239117)
--- gcc/cfgloopmanip.c  (working copy)
*** has_preds_from_loop (basic_block block,
*** 1497,1503 
  basic_block
  create_preheader (struct loop *loop, int flags)
  {
!   edge e, fallthru;
basic_block dummy;
int nentry = 0;
bool irred = false;
--- 1497,1503 
  basic_block
  create_preheader (struct loop *loop, int flags)
  {
!   edge e;
basic_block dummy;
int nentry = 0;
bool irred = false;
*** create_preheader (struct loop *loop, int
*** 1544,1552 
  
mfb_kj_edge = loop_latch_edge (loop);
latch_edge_was_fallthru = (mfb_kj_edge->flags & EDGE_FALLTHRU) != 0;
!   fallthru = make_forwarder_block (loop->header, mfb_keep_just, NULL);
!   dummy = fallthru->src;
!   loop->header = fallthru->dest;
  
/* Try to be clever in placing the newly created preheader.  The idea is to
   avoid breaking any "fallthruness" relationship between blocks.
--- 1544,1557 
  
mfb_kj_edge = loop_latch_edge (loop);
latch_edge_was_fallthru = (mfb_kj_edge->flags & EDGE_FALLTHRU) != 0;
!   if (nentry == 1)
! dummy = split_edge (single_entry);
!   else
! {
!   edge fallthru = make_forwarder_block (loop->header, mfb_keep_just, 
NULL);
!   dummy = fallthru->src;
!   loop->header = fallthru->dest;
! }
  
/* Try to be clever in placing the newly created preheader.  The idea is to
   avoid breaking any "fallthruness" relationship between blocks.
Index: gcc/testsuite/gcc.dg/tree-ssa/ldist-24.c
===
*** gcc/testsuite/gcc.dg/tree-ssa/ldist-24.c(revision 0)
--- gcc/testsuite/gcc.dg/tree-ssa/ldist-24.c(working copy)
***
*** 0 
--- 1,19 
+ /* { dg-do compile } */
+ /* { dg-options "-O3 -fdump-tree-ldist" } */
+ 
+ int foo (int flag, char *a)
+ {
+   short i, j;
+   short l = 0;
+   if (flag == 1)
+ l = 3;
+ 
+   for (i = 0; i < 4; i++)
+ {
+   for (j = l - 1; j > 0; j--)
+   a[j] = a[j - 1];
+   a[0] = i;
+ }
+ }
+ 
+ /* { dg-final { scan-tree-dump "memmove" "ldist" } } */
Index: gcc/testsuite/gcc.dg/graphite/pr35356-1.c
===
--- gcc/testsuite/gcc.dg/graphite/pr35356-1.c   (revision 239117)
+++ gcc/testsuite/gcc.dg/graphite/pr35356-1.c   (working copy)
@@ -34,4 +34,4 @@ if (n >= k + 1 && k >= 0) {
 
 */
 
-/* { dg-final { scan-tree-dump "if \\\(P_9 >= P_10 \\\+ 1 && P_10 >= 0\\\) 
\\\{" "graphite" } } */
+/* { dg-final { scan-tree-dump "if \\\(P_8 >= P_9 \\\+ 1 && P_9 >= 0\\\) \\\{" 
"graphite" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/pr59597.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr59597.c (revision 239164)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr59597.c (working copy)
@@ -54,5 +54,6 @@ main (int argc, char argv[])
   return crc;
 }
 
-/* { dg-final { scan-tree-dump "Cancelling" "vrp1" } } */
-
+/* { dg-final { scan-tree-dump-times "Registering jump thread" 3 "vrp1" } } */
+/* { dg-final { scan-tree-dump-not "joiner" "vrp1" } } */
+/* { dg-final { scan-tree-dump-times "Threaded jump" 3 "vrp1" } } */