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.