cfgcleanup: Support merging forwarder blocks with phis [PR122493]

This adds support for merging forwarder blocks with phis in cleanupcfg.
This patch might seem small but that is because the previous patches were
done to build up to make it easier to add this support.

There is still one more patch to merge remove_forwarder_block
and remove_forwarder_block_with_phi since remove_forwarder_block_with_phi
supports splitting an edge which is not supported as an option in remove_forwarder_block.
The splitting edge option should not be enabled for cfgcleanup but only for mergephi.

Note r8-338-ge7d70c6c3bccb2 added always creating a preheader for loops so we should
protect them if we have a phi node as it goes back and forth here. And both the gimple
and RTL loop code likes to have this preheader in the case of having the same constant
value being starting of the loop.

explaination on testcase changes
gcc.target/i386/pr121062-1.c needed a small change because there is a basic block
which is not duplicated so only one `movq reg, -1` is there instead of 2.

uninit-pred-7_a.c is xfailed and filed as PR122660, some analysis in the PR already of
the difference now.

uninit-pred-5.C was actually a false positive because when
m_best_candidate is non-NULL, m_best_candidate_len is always initialized.
The log message on the testcase is wrong if you manually fall the path
you can notice that. With an extra jump threading after the merging of
some bbs, the false positive is now no longer happening. So change the
dg-warning to dg-bogus.

ssa-dom-thread-7.c now jump threads 12 times in thread2 instead of 8

Bootstrapped and tested on x86_64-linux-gnu.

	PR tree-optimization/122493
gcc/ChangeLog:

	* tree-cfgcleanup.cc (tree_forwarder_block_p): Change bool argument
	to a must have phi and allow phis if it is false.
	(remove_forwarder_block): Add support for merging of forwarder blocks
	with phis.

gcc/testsuite/ChangeLog:

	* gcc.target/i386/pr121062-1.c: Update count.
	* gcc.dg/uninit-pred-7_a.c: xfail line 23.
	* g++.dg/uninit-pred-5.C: Change dg-warning to dg-bogus.
	* gcc.dg/tree-ssa/ssa-dom-thread-7.c: Update count of jump thread.

Signed-off-by: Andrew Pinski <andrew.pinski@oss.qualcomm.com>

diff --git a/gcc/testsuite/g++.dg/uninit-pred-5.C b/gcc/testsuite/g++.dg/uninit-pred-5.C
index 8dfd9874f65b043b6862fcfa4a206b99a4ffa90d..cffac6b0c1ea9a4b103502cb0080bf6df91cdb57 100644 (file)
--- a/gcc/testsuite/g++.dg/uninit-pred-5.C
+++ b/gcc/testsuite/g++.dg/uninit-pred-5.C
@@ -41,7 +41,7 @@ public:
     edit_distance_t __trans_tmp_1;
     if (m_best_candidate) {
       size_t candidate_len = m_best_candidate_len;
-      __trans_tmp_1 = get_edit_distance_cutoff(candidate_len); // { dg-warning "may be used uninitialized" }
+      __trans_tmp_1 = get_edit_distance_cutoff(candidate_len); // { dg-bogus "may be used uninitialized" }
     }
     edit_distance_t cutoff = __trans_tmp_1;
     if (cutoff)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-7.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-7.c
index 1c2cfa4fa8d53cc819d65355087dafc486ec5b0c..81bb7fc9d1e0d21d8bfd300c1e20c3828c397492 100644 (file)
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-7.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-7.c
@@ -11,8 +11,8 @@
    to change decisions in switch expansion which in turn can expose new
    jump threading opportunities.  Skip the later tests on aarch64.  */
 /* { dg-final { scan-tree-dump-not "Jumps threaded"  "dom3" { target { ! aarch64*-*-* } } } } */
-/* { dg-final { scan-tree-dump "Jumps threaded: 8"  "thread2" { target { ! aarch64*-*-* } } } } */
-/* { dg-final { scan-tree-dump "Jumps threaded: 8"  "thread2" { target { aarch64*-*-* } } } } */
+/* { dg-final { scan-tree-dump "Jumps threaded: 12"  "thread2" { target { ! aarch64*-*-* } } } } */
+/* { dg-final { scan-tree-dump "Jumps threaded: 12"  "thread2" { target { aarch64*-*-* } } } } */
 
 enum STATE {
   S0=0,

diff --git a/gcc/testsuite/gcc.dg/uninit-pred-7_a.c b/gcc/testsuite/gcc.dg/uninit-pred-7_a.c
index c2ba2a4248d707b8d2bd6d112d525c4d89984c8a..7aaadf7def752c0c503e8e0142fac59e37b41d4f 100644 (file)
--- a/gcc/testsuite/gcc.dg/uninit-pred-7_a.c
+++ b/gcc/testsuite/gcc.dg/uninit-pred-7_a.c
@@ -20,7 +20,7 @@ int foo (int n, int l, int m, int r)
       blah(v); /* { dg-bogus "uninitialized" "bogus warning" } */
 
   if ( n )
-      blah(v); /* { dg-bogus "uninitialized" "bogus warning" } */
+      blah(v); /* { dg-bogus "uninitialized" "bogus warning" { xfail *-*-* } } */
 
   if ( l )
       blah(v); /* { dg-bogus "uninitialized" "bogus warning" } */

diff --git a/gcc/testsuite/gcc.target/i386/pr121062-1.c b/gcc/testsuite/gcc.target/i386/pr121062-1.c
index 799f8562c9f7da2708d149082fa8c2f0b6b5564a..c334df64cdcad2a16ad99b9c3b3642e9fa50cf12 100644 (file)
--- a/gcc/testsuite/gcc.target/i386/pr121062-1.c
+++ b/gcc/testsuite/gcc.target/i386/pr121062-1.c
@@ -31,4 +31,4 @@ render_result_from_bake_h(int tx)
   }
 }
 
-/* { dg-final { scan-assembler-times "movq\[ \\t\]+\\\$-1, %r\[a-z0-9\]+" 2 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times "movq\[ \\t\]+\\\$-1, %r\[a-z0-9\]+" 1 { target { ! ia32 } } } } */

diff --git a/gcc/tree-cfgcleanup.cc b/gcc/tree-cfgcleanup.cc
index 803d8599b5775a37ca6bd1f1372d90c494973594..636a546a3dc92f991b116f25163bfdd46f4d2ea3 100644 (file)
--- a/gcc/tree-cfgcleanup.cc
+++ b/gcc/tree-cfgcleanup.cc
@@ -386,16 +386,13 @@ cleanup_control_flow_bb (basic_block bb)
    the entry block.  */
 
 static bool
-tree_forwarder_block_p (basic_block bb, bool phi_wanted)
+tree_forwarder_block_p (basic_block bb, bool must_have_phis)
 {
   gimple_stmt_iterator gsi;
   location_t locus;
 
   /* BB must have a single outgoing edge.  */
   if (!single_succ_p (bb)
-      /* If PHI_WANTED is false, BB must not have any PHI nodes.
-        Otherwise, BB must have PHI nodes.  */
-      || gimple_seq_empty_p (phi_nodes (bb)) == phi_wanted
       /* BB may not be a predecessor of the exit block.  */
       || single_succ (bb) == EXIT_BLOCK_PTR_FOR_FN (cfun)
       /* Nor should this be an infinite loop.  */
@@ -404,6 +401,10 @@ tree_forwarder_block_p (basic_block bb, bool phi_wanted)
       || (single_succ_edge (bb)->flags & EDGE_ABNORMAL))
     return false;
 
+  /* If MUST_HAVE_PHIS is true and we don't have any phis, return false. */
+  if (must_have_phis && gimple_seq_empty_p (phi_nodes (bb)))
+    return false;
+
   gcc_checking_assert (bb != ENTRY_BLOCK_PTR_FOR_FN (cfun));
 
   locus = single_succ_edge (bb)->goto_locus;
@@ -520,8 +521,10 @@ tree_forwarder_block_p (basic_block bb, bool phi_wanted)
                      || loops_state_satisfies_p
                           (LOOPS_MAY_HAVE_MULTIPLE_LATCHES));
            }
+         /* cleanup_tree_cfg_noloop just created the loop preheader, don't
+            remove it if it has phis.  */
          else if (bb->loop_father == loop_outer (dest->loop_father))
-           return !loops_state_satisfies_p (LOOPS_HAVE_PREHEADERS);
+           return gimple_seq_empty_p (phi_nodes (bb));
          /* Always preserve other edges into loop headers that are
             not simple latches or preheaders.  */
          return false;
@@ -625,6 +628,7 @@ remove_forwarder_block (basic_block bb)
   basic_block dest = succ->dest;
   gimple *stmt;
   gimple_stmt_iterator gsi, gsi_to;
+  bool has_phi = !gimple_seq_empty_p (phi_nodes (bb));
 
   /* If there is an abnormal edge to basic block BB, but not into
      dest, problems might occur during removal of the phi node at out
@@ -636,15 +640,24 @@ remove_forwarder_block (basic_block bb)
      does not like it.
 
      So if there is an abnormal edge to BB, proceed only if there is
-     no abnormal edge to DEST and there are no phi nodes in DEST.  */
+     no abnormal edge to DEST and there are no phi nodes in DEST.
+     If the BB has phi, we don't want to deal with abnormal edges either. */
   if (bb_has_abnormal_pred (bb)
       && (bb_has_abnormal_pred (dest)
-         || !gimple_seq_empty_p (phi_nodes (dest))))
+         || !gimple_seq_empty_p (phi_nodes (dest))
+         || has_phi))
+    return false;
+
+  /* When we have a phi, we have to feed into another
+     basic block with PHI nodes.  */
+  if (has_phi && gimple_seq_empty_p (phi_nodes (dest)))
     return false;
 
   /* If there are phi nodes in DEST, and some of the blocks that are
      predecessors of BB are also predecessors of DEST, check that the
-     phi node arguments match.  */
+     phi node arguments match.
+     Otherwise we have to split the edge and that becomes
+     a "forwarder" again.  */
   if (!gimple_seq_empty_p (phi_nodes (dest)))
     {
       edge_iterator ei;
@@ -680,9 +693,21 @@ remove_forwarder_block (basic_block bb)
 
       if (s == e)
        {
+         /* If we merge the forwarder with phis into a loop header
+            verify if we are creating another loop latch edge.
+            If so, reset number of iteration information of the loop.  */
+         if (has_phi
+             && dest->loop_father
+             && dest->loop_father->header == dest
+             && dominated_by_p (CDI_DOMINATORS, e->src, dest))
+           {
+             dest->loop_father->any_upper_bound = false;
+             dest->loop_father->any_likely_upper_bound = false;
+             free_numbers_of_iterations_estimates (dest->loop_father);
+           }
          /* Copy arguments for the phi nodes, since the edge was not
             here before.  */
-         copy_phi_arg_into_existing_phi (succ, s);
+         copy_phi_arg_into_existing_phi (succ, s, has_phi);
        }
     }