Re: [PATCH] Fix devirtualization ICE (PR tree-optimization/59622, take 3)

[Richard Biener <rguenther at suse dot de>]

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.