Re: [PR50672, PATCH] Fix ice triggered by -ftree-tail-merge: verify_ssa failed: no immediate_use list

[Tom de Vries <Tom_deVries at mentor dot com>]

On 10/12/2011 02:19 PM, Richard Guenther wrote:
> On Wed, Oct 12, 2011 at 8:35 AM, Tom de Vries <vries@codesourcery.com> wrote:
>> Richard,
>>
>> I have a patch for PR50672.
>>
>> When compiling the testcase from the PR with -ftree-tail-merge, the scenario is
>> as follows:
>>
>> We start out tail_merge_optimize with blocks 14 and 20, which are alike, but not
>> equal, since they have different successors:
>> ...
>>  # BLOCK 14 freq:690
>>  # PRED: 25 [61.0%]  (false,exec)
>>
>>  if (wD.2197_57(D) != 0B)
>>    goto <bb 15>;
>>  else
>>    goto <bb 16>;
>>  # SUCC: 15 [78.4%]  (true,exec) 16 [21.6%]  (false,exec)
>>
>>
>>  # BLOCK 20 freq:2900
>>  # PRED: 29 [100.0%]  (fallthru) 31 [100.0%]  (fallthru)
>>
>>  # .MEMD.2447_209 = PHI <.MEMD.2447_125(29), .MEMD.2447_129(31)>
>>  if (wD.2197_57(D) != 0B)
>>    goto <bb 5>;
>>  else
>>    goto <bb 6>;
>>  # SUCC: 5 [85.0%]  (true,exec) 6 [15.0%]  (false,exec)
>> ...
>>
>> In the first iteration, we merge block 5 with block 15 and block 6 with block
>> 16. After that, the blocks 14 and 20 are equal.
>>
>> In the second iteration, the blocks 14 and 20 are merged, by redirecting the
>> incoming edges of block 20 to block 14, and removing block 20.
>>
>> Block 20 also contains the definition of .MEMD.2447_209. Removing the definition
>> delinks the vuse of .MEMD.2447_209 in block 5:
>> ...
>>  # BLOCK 5 freq:6036
>>  # PRED: 20 [85.0%]  (true,exec)
>>
>>  # PT = nonlocal escaped
>>  D.2306_58 = &thisD.2200_10(D)->D.2156;
>>  # .MEMD.2447_132 = VDEF <.MEMD.2447_209>
>>  # USE = anything
>>  # CLB = anything
>>  drawLineD.2135 (D.2306_58, wD.2197_57(D), gcD.2198_59(D));
>>  goto <bb 17>;
>>  # SUCC: 17 [100.0%]  (fallthru,exec)
>> ...
> 
> And block 5 is retained and block 15 is discarded?
> 

Indeed.

>> After the pass, when executing the TODO_update_ssa_only_virtuals, we update the
>> drawLine call in block 5 using rewrite_update_stmt, which calls
>> maybe_replace_use for the vuse operand.
>>
>> However, maybe_replace_use doesn't have an effect since the old vuse and the new
>> vuse happen to be the same (rdef == use), so SET_USE is not called and the vuse
>> remains delinked:
>> ...
>>  if (rdef && rdef != use)
>>    SET_USE (use_p, rdef);
>> ...
>>
>> The patch fixes this by forcing SET_USE for delinked uses.
> 
> That isn't the correct fix.  Whoever unlinks the vuse (by removing its
> definition) has to replace it with something valid, which is either the
> bare symbol .MEM, or the VUSE associated with the removed VDEF
> (thus, as unlink_stmt_vdef does).
> 

Another try. For each deleted bb, we call unlink_stmt_vdef for the statements,
and replace the .MEM phi uses with the bare .MEM symbol.

Bootstrapped and reg-tested on x86_64.

Ok for trunk?

Thanks,
- Tom

> Richard.
> 


2011-10-14  Tom de Vries  <tom@codesourcery.com>

	PR tree-optimization/50672
	* tree-ssa-tail-merge.c (release_vdefs): New function.
	(purge_bbs): Add update_vops parameter.  Call release_vdefs for each
	deleted basic block.
	(tail_merge_optimize): Add argument to call to purge_bbs.

Index: gcc/tree-ssa-tail-merge.c
===================================================================
--- gcc/tree-ssa-tail-merge.c (revision 179773)
+++ gcc/tree-ssa-tail-merge.c (working copy)
@@ -773,18 +773,53 @@ same_succ_flush_bbs (bitmap bbs)
     }
 }
 
+/* Release all vdefs in BB, either normal or phi results.  */
+
+static void
+release_vdefs (basic_block bb)
+{
+  gimple_stmt_iterator i;
+
+  for (i = gsi_start_phis (bb); !gsi_end_p (i); gsi_next (&i))
+    {
+      gimple phi = gsi_stmt (i);
+      tree res = gimple_phi_result (phi);
+      use_operand_p use_p;
+      imm_use_iterator iter;
+      gimple use_stmt;
+
+      if (is_gimple_reg (res))
+	continue;
+
+      FOR_EACH_IMM_USE_STMT (use_stmt, iter, res)
+	{
+	  FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
+	    SET_USE (use_p, SSA_NAME_VAR (res));
+	}
+    }
+  
+  for (i = gsi_last_bb (bb); !gsi_end_p (i); gsi_prev_nondebug (&i))
+    unlink_stmt_vdef (gsi_stmt (i));
+}
+
 /* Delete all deleted_bbs.  */
 
 static void
-purge_bbs (void)
+purge_bbs (bool update_vops)
 {
   unsigned int i;
   bitmap_iterator bi;
+  basic_block bb;
 
   same_succ_flush_bbs (deleted_bbs);
 
   EXECUTE_IF_SET_IN_BITMAP (deleted_bbs, 0, i, bi)
-    delete_basic_block (BASIC_BLOCK (i));
+    {
+      bb = BASIC_BLOCK (i);
+      if (!update_vops)
+	release_vdefs (bb);
+      delete_basic_block (bb);
+    }
 
   bitmap_and_compl_into (deleted_bb_preds, deleted_bbs);
   bitmap_clear (deleted_bbs);
@@ -1665,7 +1700,7 @@ tail_merge_optimize (unsigned int todo)
 	break;
 
       free_dominance_info (CDI_DOMINATORS);
-      purge_bbs ();
+      purge_bbs (update_vops);
 
       if (iteration_nr == max_iterations)
 	break;