I am testing the following patch to avoid CFG cleanup calling merge-blocks
in dead regions thereby exposing that (in this case) copyprop propagates
a def from a dead region to another place in another dead region
(itself not realizing they are dead).  The issue with that is that
if CFG cleanup first removes the region with the def and later
tries to propagate out a PHI merging two blocks (in the other dead
region, before finally removing it) it ICEs because may_propagate_copy
is not happy with being called on released SSA names.

The above scheme can only happen if a propagator created "invalid"
SSA form by viewing some edges as non-executable thereby making
the use being dominated by the def only "virtually".  It forces
non-executable edges to be optimized away by CFG cleanup by making
the controlling conditions trivially true/false.

The fix is to make the SSA form "valid" as a very first thing in
CFG cleanup, thus remove trivially dead edges before trying to
execute BB merging opportunities.

Eventually we can avoid iterating cleanup_control_flow_bb (secondary
opportunities should be very rare) but I didn't want to change
this during stage3.  I'll place a gcc_assert there in another
bootstrap anyway (in theory out-propagating a PHI during BB merging
could expose a trivially true/false condition though BB merging)

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

Richard.

2015-12-01  Richard Biener  <rguent...@suse.de>

        PR tree-optimization/68625
        * tree-cfgcleanup.c (cleanup_tree_cfg_bb): Do not call
        cleanup_control_flow_bb.
        (cleanup_tree_cfg_1): First perform cleanup_control_flow_bb
        on all BBs, then cleanup_tree_cfg_bb and finally iterate
        over the worklist doing both.

        * gcc.dg/torture/pr68625.c: New testcase.

Index: gcc/tree-cfgcleanup.c
===================================================================
*** gcc/tree-cfgcleanup.c       (revision 231101)
--- gcc/tree-cfgcleanup.c       (working copy)
*************** fixup_noreturn_call (gimple *stmt)
*** 614,621 ****
  static bool
  cleanup_tree_cfg_bb (basic_block bb)
  {
-   bool retval = cleanup_control_flow_bb (bb);
- 
    if (tree_forwarder_block_p (bb, false)
        && remove_forwarder_block (bb))
      return true;
--- 614,619 ----
*************** cleanup_tree_cfg_bb (basic_block bb)
*** 640,646 ****
        }
      }
  
!   return retval;
  }
  
  /* Iterate the cfg cleanups, while anything changes.  */
--- 638,644 ----
        }
      }
  
!   return false;
  }
  
  /* Iterate the cfg cleanups, while anything changes.  */
*************** cleanup_tree_cfg_1 (void)
*** 660,667 ****
       recording of edge to CASE_LABEL_EXPR.  */
    start_recording_case_labels ();
  
!   /* Start by iterating over all basic blocks.  We cannot use FOR_EACH_BB_FN,
       since the basic blocks may get removed.  */
    n = last_basic_block_for_fn (cfun);
    for (i = NUM_FIXED_BLOCKS; i < n; i++)
      {
--- 658,683 ----
       recording of edge to CASE_LABEL_EXPR.  */
    start_recording_case_labels ();
  
!   /* We cannot use FOR_EACH_BB_FN for the BB iterations below
       since the basic blocks may get removed.  */
+ 
+   /* Start by iterating over all basic blocks looking for edge removal
+      opportunities.  Do this first because incoming SSA form may be
+      invalid and we want to avoid performing SSA related tasks such
+      as propgating out a PHI node during BB merging in that state.  */
+   n = last_basic_block_for_fn (cfun);
+   for (i = NUM_FIXED_BLOCKS; i < n; i++)
+     {
+       bb = BASIC_BLOCK_FOR_FN (cfun, i);
+       if (bb)
+       retval |= cleanup_control_flow_bb (bb);
+     }
+ 
+   /* After doing the above SSA form should be valid (or an update SSA
+      should be required).  */
+ 
+   /* Continue by iterating over all basic blocks looking for BB merging
+      opportunities.  */
    n = last_basic_block_for_fn (cfun);
    for (i = NUM_FIXED_BLOCKS; i < n; i++)
      {
*************** cleanup_tree_cfg_1 (void)
*** 682,687 ****
--- 698,704 ----
        if (!bb)
        continue;
  
+       retval |= cleanup_control_flow_bb (bb);
        retval |= cleanup_tree_cfg_bb (bb);
      }
  
Index: gcc/testsuite/gcc.dg/torture/pr68625.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/pr68625.c      (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr68625.c      (working copy)
***************
*** 0 ****
--- 1,51 ----
+ /* { dg-do compile } */
+ /* { dg-additional-options "-w" } */
+ 
+ int **dp;
+ int sg;
+ 
+ void
+ z9(void)
+ {
+   int pz, oi, vz, yp, zi, hd, pw, gr, w9 = 0, j0 = -1, rb = &w9;
+   int *lr;
+   while (w9 < 1) {
+       lr++;
+       *lr = 1;
+       if (*lr < 1)
+       for (;;)
+         if (pz && *lr) {
+ ee:
+             **dp = 0;
+         }
+       pz = zi = vz;
+       if (j0 ^ (vz > 0))
+       continue;
+       **dp = 1;
+       while (**dp)
+       if (++oi) {
+           int mq = dp;
+           j0 = 1;
+           while (pw < 1) {
+               if (++rb && mq)
+                 xq:
+                     hd = sg;
+               ++pw;
+           }
+           sg = 0;
+           while (!sg) {
+               goto ee;
+               while (++yp && gr++) {
+                   int i9, xa;
+                   while (++i9 && ++xa)
+                     fb:
+                         ;
+               }
+           }
+       }
+   }
+   ++vz;
+   if (zi > hd)
+     goto xq;
+   goto fb;
+ }

Reply via email to