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

login
register
mail settings
Submitter Tom de Vries
Date Oct. 12, 2011, 6:35 a.m.
Message ID <4E953540.3010807@codesourcery.com>
Download mbox | patch
Permalink /patch/119147/
State New
Headers show

Comments

Tom de Vries - Oct. 12, 2011, 6:35 a.m.
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)
...

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.

Bootstrapped and reg-tested on x86_64.

OK for trunk?

Thanks,
- Tom

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

	PR tree-optimization/50672
	* tree-into-ssa.c (maybe_replace_use): Force SET_USE for delinked uses.
Richard Guenther - Oct. 12, 2011, 12:19 p.m.
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?

> 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).

Richard.

> Bootstrapped and reg-tested on x86_64.
>
> OK for trunk?
>
> Thanks,
> - Tom
>
> 2011-10-12  Tom de Vries  <tom@codesourcery.com>
>
>        PR tree-optimization/50672
>        * tree-into-ssa.c (maybe_replace_use): Force SET_USE for delinked uses.
>

Patch

Index: gcc/tree-into-ssa.c
===================================================================
--- gcc/tree-into-ssa.c (revision 179592)
+++ gcc/tree-into-ssa.c (working copy)
@@ -1908,7 +1908,7 @@  maybe_replace_use (use_operand_p use_p)
   else if (is_old_name (use))
     rdef = get_reaching_def (use);
 
-  if (rdef && rdef != use)
+  if (rdef && (rdef != use || (!use_p->next && !use_p->prev)))
     SET_USE (use_p, rdef);
 }