Re: [PATCH, PR50763] Fix for ICE in verify_gimple

[Richard Guenther <richard.guenther@gmail.com>]

On Thu, Oct 20, 2011 at 3:48 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> Richard,
>
> I have a fix for PR50763.
>
> The second example from the PR looks like this:
> ...
> int bar (int i);
>
> void
> foo (int c, int d)
> {
>  if (bar (c))
>    bar (c);
>  d = 33;
>  while (c == d);
> }
> ...
>
> When compiled with -O2 -fno-dominator-opt, the gimple representation before
> ftree-tail-merge looks like this:
> ...
> foo (intD.6 cD.1606, intD.6 dD.1607)
> {
>  intD.6 D.2730;
>
>  # BLOCK 2 freq:900
>  # PRED: ENTRY [100.0%]  (fallthru,exec)
>  # .MEMD.2733_6 = VDEF <.MEMD.2733_5(D)>
>  # USE = nonlocal
>  # CLB = nonlocal
>  D.2730_2 = barD.1605 (cD.1606_1(D));
>  if (D.2730_2 != 0)
>    goto <bb 3>;
>  else
>    goto <bb 7>;
>  # SUCC: 3 [29.0%]  (true,exec) 7 [71.0%]  (false,exec)
>
>  # BLOCK 7 freq:639
>  # PRED: 2 [71.0%]  (false,exec)
>  goto <bb 4>;
>  # SUCC: 4 [100.0%]  (fallthru)
>
>  # BLOCK 3 freq:261
>  # PRED: 2 [29.0%]  (true,exec)
>  # .MEMD.2733_7 = VDEF <.MEMD.2733_6>
>  # USE = nonlocal
>  # CLB = nonlocal
>  barD.1605 (cD.1606_1(D));
>  # SUCC: 4 [100.0%]  (fallthru,exec)
>
>  # BLOCK 4 freq:900
>  # PRED: 7 [100.0%]  (fallthru) 3 [100.0%]  (fallthru,exec)
>  # .MEMD.2733_4 = PHI <.MEMD.2733_6(7), .MEMD.2733_7(3)>
>  if (cD.1606_1(D) == 33)
>    goto <bb 8>;
>  else
>    goto <bb 9>;
>  # SUCC: 8 [91.0%]  (true,exec) 9 [9.0%]  (false,exec)
>
>  # BLOCK 9 freq:81
>  # PRED: 4 [9.0%]  (false,exec)
>  goto <bb 6>;
>  # SUCC: 6 [100.0%]  (fallthru)
>
>  # BLOCK 8 freq:819
>  # PRED: 4 [91.0%]  (true,exec)
>  # SUCC: 5 [100.0%]  (fallthru)
>
>  # BLOCK 5 freq:9100
>  # PRED: 8 [100.0%]  (fallthru) 10 [100.0%]  (fallthru)
>  if (cD.1606_1(D) == 33)
>    goto <bb 10>;
>  else
>    goto <bb 11>;
>  # SUCC: 10 [91.0%]  (true,exec) 11 [9.0%]  (false,exec)
>
>  # BLOCK 10 freq:8281
>  # PRED: 5 [91.0%]  (true,exec)
>  goto <bb 5>;
>  # SUCC: 5 [100.0%]  (fallthru)
>
>  # BLOCK 11 freq:819
>  # PRED: 5 [9.0%]  (false,exec)
>  # SUCC: 6 [100.0%]  (fallthru)
>
>  # BLOCK 6 freq:900
>  # PRED: 11 [100.0%]  (fallthru) 9 [100.0%]  (fallthru)
>  # VUSE <.MEMD.2733_4>
>  return;
>  # SUCC: EXIT [100.0%]
>
> }
> ...
>
> During the first iteration, tail_merge_optimize finds that block 9 and 11, and
> block 8 and 10 are equal, and removes block 11 and 10.
> During the second iteration it finds that block 4 and block 5 are equal, and it
> removes block 5.
>
> Since pre had no effect, the responsibility for updating the vops lies with
> tail_merge_optimize.
>
> Block 4 starts with a virtual PHI which needs updating, but replace_block_by
> decides that an update is not necessary, because vop_at_entry returns NULL_TREE
> for block 5 (the vop_at_entry for block 4 is .MEMD.2733_4).
> What is different from normal is that block 4 dominates block 5.
>
> The patch makes sure that the vops are also updated if vop_at_entry is defined
> for only one of bb1 and bb2.
>
> This also forced me to rewrite the code that updates the uses, which uses
> dominator info now. This forced me to keep the dominator info up-to-date. Which
> forced me to move the actual deletion of the basic block and some additional
> bookkeeping related to that from purge_bbs to replace_block_by.
>
> Additionally, I fixed the case that update_vuses leaves virtual phis with only
> one argument (see unlink_virtual_phi).
>
> bootstrapped and reg-tested on x86_64. The tested patch had one addition to the
> attached patch: calling verify_dominators at the end of replace_block_by.
>
> OK for trunk?

+      if (gimple_code (stmt) != GIMPLE_PHI &&
+         !dominated_by_p (CDI_DOMINATORS, gimple_bb (stmt), bb2))
          continue;

&&s go to the next line please.

The unlink_virtual_phi function needs a comment.

Ok with that changes.

Richard.

> Thanks,
> - Tom
>
> 2011-10-20  Tom de Vries  <tom@codesourcery.com>
>
>        PR tree-optimization/50763
>        * tree-ssa-tail-merge.c (same_succ_flush_bb): New function, factored out
>        of ...
>        (same_succ_flush_bbs): Use same_succ_flush_bb.
>        (purge_bbs): Remove argument.  Remove calls to same_succ_flush_bbs,
>        release_last_vdef and delete_basic_block.
>        (unlink_virtual_phi): New function.
>        (update_vuses): Add and use vuse1_phi_args argument.  Set var to
>        SSA_NAME_VAR of vuse1 or vuse2, and use var.  Handle case that def_stmt2
>        is NULL.  Use phi result as phi arg in case vuse1 or vuse2 is NULL_TREE.
>        Replace uses of vuse1 if vuse2 is NULL_TREE.  Fix code to limit
>        replacement of uses.  Propagate phi argument for phis with a single
>        argument.
>        (replace_block_by): Update vops if phi_vuse1 or phi_vuse2 is NULL_TREE.
>        Set vuse1_phi_args if vuse1 is a phi defined in bb1.  Add vuse1_phi_args
>        as argument to call to update_vuses.  Call release_last_vdef,
>        same_succ_flush_bb, delete_basic_block.  Update CDI_DOMINATORS info.
>        (tail_merge_optimize): Remove argument in call to purge_bbs.  Remove
>        call to free_dominance_info.  Only call calculate_dominance_info once.
>
>        * gcc.dg/pr50763.c: New test.
>