Patchwork : Fix compiler segfault failure in cd_dce pass

login
register
mail settings
Submitter Xinliang David Li
Date Nov. 21, 2012, 9:35 p.m.
Message ID <CAAkRFZJnb+Gz0kr7Ss6x_2EhgF-KOiVDhJv5w_ZDDTLQSn=m+g@mail.gmail.com>
Download mbox | patch
Permalink /patch/200864/
State New
Headers show

Comments

Xinliang David Li - Nov. 21, 2012, 9:35 p.m.
In compiling one of the very large C++ source, the compiler hit a
segfault in cddce -- the ssa_name of a vuse operand has a null def
stmt.

The def stmt was a PHI node, and later got removed by the phicprop
pass (in eliminate_degenerated_phis because it seems to have zero
uses)

<bb 17>:
 .MEM_343 =  PHI (.......)
  VUSE (.MEM_343);
  std::__throw_runtime_error (..)


From the IR dump, it looks normal -- the PHI's def has a real use, but
why does it got removed? Closer investigation shows that there is
actually no use operand created for MEM_343 and the VUSE is a dangling
reference. How did this happen?

Here is how it happens:

The phi node was created during SSA update after PRE, The
make_ssa_name happens to pick up the dead ssa_name MEM_343 just got
released because of unreachable code elimination (note that PRE can
make condition be folded). After MEM_343 is release, the virtual use
in bb 17 becomes dangled ( bb 17 was not deleted together with its
predecessor that defines MEM_343 is because  of the tail block merging
pass in PRE).  During post PRE ssa_update, the compiler sees that the
tree value is the same '.MEM_343' so it does not bother to recreate
the use, thus makes the phi look like dead.


(As you can see, the condition to trigger the failure is really really
rare, that is why there is no reduced test case for it --- multidelta
is crunching for 4 days, and the best it got is still > 50k lines).


The following simple patch solve the problem. Bootstrtapped and there
is no regression. Ok to install for trunk?



2012-11-21  Xinliang David Li  <davidxl@google.com>

* tree-into-ssa.c (make_replace_use): force use replacement
in SSA update.


David
Richard Guenther - Nov. 25, 2012, 1:28 p.m.
On Wed, Nov 21, 2012 at 10:35 PM, Xinliang David Li <davidxl@google.com> wrote:
> In compiling one of the very large C++ source, the compiler hit a
> segfault in cddce -- the ssa_name of a vuse operand has a null def
> stmt.
>
> The def stmt was a PHI node, and later got removed by the phicprop
> pass (in eliminate_degenerated_phis because it seems to have zero
> uses)
>
> <bb 17>:
>  .MEM_343 =  PHI (.......)
>   VUSE (.MEM_343);
>   std::__throw_runtime_error (..)
>
>
> From the IR dump, it looks normal -- the PHI's def has a real use, but
> why does it got removed? Closer investigation shows that there is
> actually no use operand created for MEM_343 and the VUSE is a dangling
> reference. How did this happen?
>
> Here is how it happens:
>
> The phi node was created during SSA update after PRE, The
> make_ssa_name happens to pick up the dead ssa_name MEM_343 just got
> released because of unreachable code elimination (note that PRE can
> make condition be folded). After MEM_343 is release, the virtual use
> in bb 17 becomes dangled ( bb 17 was not deleted together with its
> predecessor that defines MEM_343 is because  of the tail block merging
> pass in PRE).  During post PRE ssa_update, the compiler sees that the
> tree value is the same '.MEM_343' so it does not bother to recreate
> the use, thus makes the phi look like dead.
>
>
> (As you can see, the condition to trigger the failure is really really
> rare, that is why there is no reduced test case for it --- multidelta
> is crunching for 4 days, and the best it got is still > 50k lines).
>
>
> The following simple patch solve the problem. Bootstrtapped and there
> is no regression. Ok to install for trunk?

That's not the correct fix - the fix is to fix PRE not to do this.
Btw, I fixed a
similar issue a few weeks ago - are you sure the problem still persists?

Thanks,
Richard.

>
>
> Index: tree-into-ssa.c
> ===================================================================
> --- tree-into-ssa.c     (revision 193698)
> +++ tree-into-ssa.c     (working copy)
> @@ -1767,7 +1767,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)
>      SET_USE (use_p, rdef);
>  }
>
> 2012-11-21  Xinliang David Li  <davidxl@google.com>
>
> * tree-into-ssa.c (make_replace_use): force use replacement
> in SSA update.
>
>
> David

Patch

Index: tree-into-ssa.c
===================================================================
--- tree-into-ssa.c     (revision 193698)
+++ tree-into-ssa.c     (working copy)
@@ -1767,7 +1767,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)
     SET_USE (use_p, rdef);
 }