On Mon, 30 Mar 2020, Jakub Jelinek wrote:
> On Mon, Mar 30, 2020 at 12:44:40PM +0200, Richard Biener wrote:
> > > type = TREE_TYPE (stmt_expr);
> > > + /* For statement-expressions, force the STATEMENT_LIST
> > > + to be preserved with side-effects, even if it contains
> > > + just one statement or no statements. See PR93786. */
> > > + TREE_SIDE_EFFECTS (stmt_expr) = 0;
> > > result = pop_stmt_list (stmt_expr);
> > > + gcc_assert (result == stmt_expr);
> > > + TREE_SIDE_EFFECTS (result) = 1;
> >
> > The original code had !TREE_SIDE_EFFECT stmt lists (empty ones?) returned
> > directly which you above change to have side-effects - are you sure
> > the fallout is not due to that? Did you look into any of the fallout?
>
> Some of the fallout has been from missed optimizations e.g. in initializers,
> where the TREE_SIDE_EFFECTs (sometimes for empty statement exprs like
> ({ ; ; ; }), at other times for one with a single real statement in there
> which didn't have side-effects) is with the patch now set when it wasn't
> before, but there were ICEs too.
I see. I was thinking of
if (TREE_SIDE_EFFECTS (stmt_expr))
{
TREE_SIDE_EFFECTS (stmt_expr) = 0;
result = pop_stmt_list (stmt_expr);
gcc_assert (result == stmt_expr);
TREE_SIDE_EFFECTS (result) = 1;
}
else
result = pop_stmt_list (stmt_expr);
which should make ({ ; ; ; }) work again, but obviously not the
single-stmt case.
> I guess another option is what Alex wrote, just throw the DEBUG_BEGIN_STMTs
> away if there would be just those or those plus a single other statement,
> perhaps limited to statement expressions (i.e. do it in the above spot).
Sure, that's another option - but of course only short-term since we
then end up with "bad" debug info.
Btw, does the above fallout already happen if you add -g? Because
all the affected stmt-lists should end up with some DEBUG_BEGIN_STMTs
in them and thus preserved?
Richard.
