On Thu, Jan 13, 2022 at 02:49:47PM +0100, Richard Biener wrote:
> > + tree d = build_debug_expr_decl (type);
> > + gdebug *g
> > + = gimple_build_debug_bind (d, build2 (rcode, type,
> > + new_lhs, arg),
> > + stmt2);
> > + gsi_insert_after (&gsi, g, GSI_NEW_STMT);
> > + replace_uses_by (lhs2, d);
>
> I wonder if you can leave a lhs2 = d; in the IL instead of using
> replace_uses_by which will process imm uses and fold stmts while
> we're going to do that anyway in the caller? That would IMHO
> be better here.
I'd need to emit them always for reversible ops and when the
atomic call can't be last, regardless of whether it is needed or not,
just so that next DCE would remove those up and emit those debug stmts,
because otherwise that could result in -fcompare-debug failures
(at least with -fno-tree-dce -fno-tree-whatever ...).
And
+ tree narg = build_debug_expr_decl (type);
+ gdebug *g
+ = gimple_build_debug_bind (narg,
+ fold_convert (type, arg),
+ stmt2);
isn't that much more code compared to
gimple *g = gimple_build_assign (lhs2, NOP_EXPR, arg);
Or would you like it to be emitted always, i.e.
if (atomic_op != BIT_AND_EXPR
&& atomic_op != BIT_IOR_EXPR
/* With -fnon-call-exceptions if we can't
add stmts after the call easily. */
&& !stmt_ends_bb_p (stmt2))
{
tree type = TREE_TYPE (lhs2);
if (TREE_CODE (arg) == INTEGER_CST)
arg = fold_convert (type, arg);
else if (!useless_type_conversion_p (type, TREE_TYPE (arg)))
{
tree narg = make_ssa_name (type);
gimple *g = gimple_build_assign (narg, NOP_EXPR, arg);
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
arg = narg;
}
enum tree_code rcode;
switch (atomic_op)
{
case PLUS_EXPR: rcode = MINUS_EXPR; break;
case MINUS_EXPR: rcode = PLUS_EXPR; break;
case BIT_XOR_EXPR: rcode = atomic_op; break;
default: gcc_unreachable ();
}
tree d = build_debug_expr_decl (type);
gimple *g = gimple_build_assign (lhs2, rcode, new_lhs, arg);
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
lhs2 = NULL_TREE;
}
in between
update_stmt (use_stmt);
and
imm_use_iterator iter;
and then do the
FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs2)
if (use_stmt != cast_stmt)
with resetting only if (lhs2)
and similarly release_ssa_name (lhs2) only if (lhs2)?
I think the usual case is that we emit debug exprs right away,
not emit something that we want to DCE.
+ if (atomic_op == BIT_AND_EXPR
+ || atomic_op == BIT_IOR_EXPR
+ /* Or with -fnon-call-exceptions if we can't
+ add debug stmts after the call. */
+ || stmt_ends_bb_p (stmt2))
But now that you mention it, I think I don't handle right the
case where lhs2 has no debug uses but there is a cast_stmt that has debug
uses for its lhs. We'd need to add_debug_temp in that case too and
add a debug temp.
Jakub
