On Fri, May 1, 2015 at 8:09 PM, Eric Botcazou <ebotca...@adacore.com> wrote: >> OK, how aggressive then? We could as well do the substitution for all >> copies: >> >> /* For EXPAND_INITIALIZER try harder to get something simpler. >> Otherwise, substitute copies on the RHS, this can propagate >> constants at -O0 and thus simplify arithmetic operations. */ >> if (g == NULL >> && !SSA_NAME_IS_DEFAULT_DEF (exp) >> && (optimize || DECL_IGNORED_P (SSA_NAME_VAR (exp))) >> && (modifier == EXPAND_INITIALIZER >> >> || (modifier != EXPAND_WRITE >> >> && gimple_assign_copy_p (SSA_NAME_DEF_STMT (exp)))) >> && stmt_is_replaceable_p (SSA_NAME_DEF_STMT (exp))) >> g = SSA_NAME_DEF_STMT (exp); > > This doesn't work (this generates wrong code because this creates overlapping > live ranges for SSA_NAMEs with the same base variable). Here's the latest > working version, all the predicates and accessors used are inlined.
Hum, the fact that your earlier version created wrong code (get_gimple_for_ssa_name already returned false here) points at some issues with EXPAND_INITIALIZER as well, no...? That said, the path you add is certainly safe (though maybe we want to change get_gimple_for_ssa_name to return tcc_constant single-use defs even if TER is disabled (thus at -O0 - and only at -O0, otherwise it shouldn't happen). That would cover more cases of get_gimple_for_ssa_name uses (I can see optimize_bitfield_expansion for example...) So, your patch is ok for trunk unless you want to explore the get_gimple_for_ssa_name improvement suggestion. I also wonder about EXPAND_INITIALIZER creating overlapping life-ranges (or moving loads across stores). Thanks, Richard. > Tested on x86_64-suse-linux, OK for the mainline? > > > 2015-05-01 Eric Botcazou <ebotca...@adacore.com> > > * expr.c (expand_expr_real_1) <SSA_NAME>: Try to substitute constants > on the RHS of expressions. > * gimple-expr.h (is_gimple_constant): Reorder. > > > -- > Eric Botcazou
