On Thu, 9 Jan 2014, Jakub Jelinek wrote: > On Thu, Jan 09, 2014 at 02:13:39PM +0100, Richard Biener wrote: > > On Thu, 9 Jan 2014, Jakub Jelinek wrote: > > > > > On Thu, Jan 09, 2014 at 01:30:53PM +0100, Richard Biener wrote: > > > > > gimplify_modify_expr has: > > > > > > > > > > if (!gimple_call_noreturn_p (assign)) > > > > > gimple_call_set_lhs (assign, *to_p); > > > > > > > > Ok, it seems to be too early then - move it after the folding. > > > > > > That wouldn't help all the other early calls of fold_stmt though. > > > E.g. lower_omp. Plus, even in gimplify_modify_expr, doing it > > > after fold_stmt would mean having to walk all stmts created by the > > > folding?, > > > check if they are calls (because a call can fold into nothing or something > > > completely different). Isn't it better then fold_stmt does that instead? > > > > Hmm, maybe. Not sure why we are this anal about requiring noreturn > > calls not to have a LHS. But if we require callers in SSA form > > to update the stmt and properly cleanup the cfg if fold_stmt returns > > true then it's reasonable to require at least "something" for callers > > from non-SSA/CFG code. > > > > That is, I don't like this special-casing. If so, then rather > > don't fold at this point - thus if (... !inplace && in_ssa_form (cfun) > > ...) (or rather if we have a CFG - cfun && cfun->curr_properties & > > PROP_cfg). > > But, isn't right now gimplification the only guaranteed folding of all > stmts?
Actually gimplification doesn't fold all stmts either, but yes. > I mean, other passes fold_stmt only if they propagate something into > them, don't they? Also, in most cases the call actually isn't noreturn, > so stopping all the devirtualization just for the unlikely case doesn't look > like a good idea to me. > > Perhaps, if you don't like the !gimple_in_ssa_p (cfun) in the condition > we can just drop the lhs always in that case, just doing what we do for > __builtin_unreachable if lhs is SSA_NAME: > tree var = create_tmp_var (TREE_TYPE (lhs), NULL); > tree def = get_or_create_ssa_default_def (cfun, var); > gsi_insert_after (gsi, gimple_build_assign (lhs, def), GSI_NEW_STMT); That works for me. Richard.
