https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116460
--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #13)
> (In reply to Andrew Pinski from comment #8)
> > ./a.ltrans6.ltrans.212t.forwprop4
> >
> > Removing dead stmt noDataCandVec$_M_start_888 = PHI <_1783(176), _577(186)>
> > ...
> > Removing dead stmt:_598 = _888 + 16;
> >
> > So it looks like we remove the statement defining _888 and then removing the
> > use.
> > The removal of _888 happens directly from forwprop while _598 definition
> > removal comes from simple_dce_from_worklist .
> >
> > The ICE happens because the ssa name _888 has already been freed so the type
> > is null (and not in this case a pointer) since this was originally a pointer
> > plus.
> >
> > Trying to reduce this further.
>
> _888 definition is from a BB which is going to be removed so we should not
> need to mark its uses as being needed for dce worklist. But I am not sure
> how to detect that case.
forwprop shouldn't remove _888 if there's a use left. When adding
simple_dce_from_worklist, did you remove some manual stmt removal
(adding to to_remove)? Having both is a bit ugly (see also
remove_prop_source_from_use), but the sets need to be separate to
avoid interactions like this.
Ah, we have
/* Substitute from our lattice. We need to do so only once. */
bool substituted_p = false;
use_operand_p usep;
ssa_op_iter iter;
FOR_EACH_SSA_USE_OPERAND (usep, stmt, iter, SSA_OP_USE)
{
tree use = USE_FROM_PTR (usep);
tree val = fwprop_ssa_val (use);
if (val && val != use)
{
if (!is_gimple_debug (stmt))
bitmap_set_bit (simple_dce_worklist, SSA_NAME_VERSION
(use));
I think that's problematic iff 'use' as definition is already in to_remove.
Note to_remove also removes in "proper" order for debug stmt generation
while simple_dce_from_worklist doesn't iff the DCEd bits are not
independent.
OK, so the issue is that when we add
noDataCandVec$_M_start_3 = PHI <_158(9), _51(12)>
to to_remove the uses are
noDataCandVec$_M_start_3 : -->5 uses.
# DEBUG __i$_M_current => noDataCandVec$_M_start_3 + 1
_56 = noDataCandVec$_M_finish_75 - noDataCandVec$_M_start_3;
ivtmp.163_15 = (unsigned long) noDataCandVec$_M_start_3;
_108 = noDataCandVec$_M_start_3 + 15;
# DEBUG __first$_M_current => noDataCandVec$_M_start_3
_57 = noDataCandVec$_M_start_3 + 16;
# DEBUG __first => noDataCandVec$_M_start_3
if (noDataCandVec$_M_start_3 != noDataCandVec$_M_finish_75)
# DEBUG __first => noDataCandVec$_M_start_3
# DEBUG __first => noDataCandVec$_M_start_3
# DEBUG noDataCandVec$_M_start => noDataCandVec$_M_start_3
and _3 is in BB 13 while the use in _57 = noDataCandVec$_M_start_3 + 16 is
in BB 15 and we are not processing this block as it is compute dead.
So the uses in dead stmts are not replaced which is what confuses
simple_dce_from_worklist.
Note it's _57 that ends up on the worklist somehow even though it's def is in
a dead BB(!?). That's from
auto_vec<tree, 8> uses;
FOR_EACH_SSA_USE_OPERAND (usep, stmt, iter, SSA_OP_USE)
if (uses.space (1))
uses.quick_push (USE_FROM_PTR (usep));
if (fold_stmt (&gsi, fwprop_ssa_val, simple_dce_worklist))
on
if (noDataCandVec$_M_start_3 != noDataCandVec$_M_finish_75)
in BB 19 which we do process. That's because we enter a loop and handle
those conservatively with respect to reachability.
We can improved this as noted in the comment:
/* With dominators we could improve backedge handling
when e->src is dominated by bb. But for irreducible
regions we have to take all backedges conservatively.
We can handle single-block cycles as we know the
dominator relationship here. */
but the latent issue remains. I'm not sure I fully got it as a simple
patch like below doesn't fix it.
diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc
index 9595555138c..cd3e68bd226 100644
--- a/gcc/tree-ssa-forwprop.cc
+++ b/gcc/tree-ssa-forwprop.cc
@@ -4143,9 +4144,18 @@ pass_forwprop::execute (function *fun)
}
gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
if (gimple_code (stmt) == GIMPLE_PHI)
- remove_phi_node (&gsi, true);
+ {
+ bitmap_clear_bit (simple_dce_worklist,
+ SSA_NAME_VERSION (gimple_phi_result (stmt)));
+ remove_phi_node (&gsi, true);
+ }
else
{
+ def_operand_p def_p;
+ ssa_op_iter op_iter;
+ FOR_EACH_SSA_DEF_OPERAND (def_p, stmt, op_iter, SSA_OP_DEF)
+ bitmap_clear_bit (simple_dce_worklist,
+ SSA_NAME_VERSION (DEF_FROM_PTR (def_p)));
unlink_stmt_vdef (stmt);
gsi_remove (&gsi, true);
release_defs (stmt);
I'm testing the following which makes the issue latent.
diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc
index 9595555138c..070299255f1 100644
--- a/gcc/tree-ssa-forwprop.cc
+++ b/gcc/tree-ssa-forwprop.cc
@@ -3536,12 +3536,11 @@ pass_forwprop::execute (function *fun)
FOR_EACH_EDGE (e, ei, bb->preds)
{
if ((e->flags & EDGE_EXECUTABLE)
- /* With dominators we could improve backedge handling
- when e->src is dominated by bb. But for irreducible
- regions we have to take all backedges conservatively.
- We can handle single-block cycles as we know the
- dominator relationship here. */
- || bb_to_rpo[e->src->index] > i)
+ /* We can handle backedges in natural loops correctly but
+ for irreducible regions we have to take all backedges
+ conservatively when we did not visit the source yet. */
+ || (bb_to_rpo[e->src->index] > i
+ && !dominated_by_p (CDI_DOMINATORS, e->src, e->dest)))
{
any = true;
break;
