Message ID | CAFiYyc2Oh-EQUpM9hYyvLU4Pu-+dHuqs-gAK79UZbXVtrdxJjA@mail.gmail.com |
---|---|
State | New |
Headers | show |
> In this case it's fold-all-builtins folding a strlen call with a > PHI <"foo", "bar"> argument. IMHO not presenting RTL with such > non-sense is best achieved by not letting TER do constant propagation > (because it doesn't "fold" the result). We can never rule out such > stray non-propagated constants, so that makes expand more robust > (and hopes for RTL CCP). > > Index: gcc/tree-ssa-ter.c > =================================================================== > --- gcc/tree-ssa-ter.c (revision 204664) > +++ gcc/tree-ssa-ter.c (working copy) > @@ -438,6 +439,12 @@ ter_is_replaceable_p (gimple stmt) > && !is_gimple_val (gimple_assign_rhs1 (stmt))) > return false; > > + /* Do not propagate "modeless" constants - we may end up > confusing the RTL > + expanders. Leave the optimization to RTL CCP. */ > + if (gimple_assign_single_p (stmt) > + && CONSTANT_CLASS_P (gimple_assign_rhs1 (stmt))) > + return false; > + > return true; > } > return false; > > does that make sense? I'll test it then. I agree with Joern that we want more constant propagation/folding during RTL expansion, not less, so IMO that's the wrong direction.
On Mon, Nov 11, 2013 at 12:21 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> In this case it's fold-all-builtins folding a strlen call with a >> PHI <"foo", "bar"> argument. IMHO not presenting RTL with such >> non-sense is best achieved by not letting TER do constant propagation >> (because it doesn't "fold" the result). We can never rule out such >> stray non-propagated constants, so that makes expand more robust >> (and hopes for RTL CCP). >> >> Index: gcc/tree-ssa-ter.c >> =================================================================== >> --- gcc/tree-ssa-ter.c (revision 204664) >> +++ gcc/tree-ssa-ter.c (working copy) >> @@ -438,6 +439,12 @@ ter_is_replaceable_p (gimple stmt) >> && !is_gimple_val (gimple_assign_rhs1 (stmt))) >> return false; >> >> + /* Do not propagate "modeless" constants - we may end up >> confusing the RTL >> + expanders. Leave the optimization to RTL CCP. */ >> + if (gimple_assign_single_p (stmt) >> + && CONSTANT_CLASS_P (gimple_assign_rhs1 (stmt))) >> + return false; >> + >> return true; >> } >> return false; >> >> does that make sense? I'll test it then. > > I agree with Joern that we want more constant propagation/folding during > RTL expansion, not less, so IMO that's the wrong direction. The question is whether you for example want to handle a_2 = 1 + 0; at RTL expansion time? I'd say it's better to have b_1 = 0; a_2 = 1 + b_1; not to say the proposed patch would be a way to ensure the first didn't happen - it just makes it less likely that TER gets you into this. OTOH as TER is now "explicit" (the expander has to lookup SSA defs) those uses should better deal with constants they receive from that. Richard. > -- > Eric Botcazou
On Mon, Nov 11, 2013 at 12:26:09PM +0100, Richard Biener wrote: > The question is whether you for example want to handle > > a_2 = 1 + 0; > > at RTL expansion time? I'd say it's better to have I think we already handle that just fine, there are tons of various simplify_gen_* calls during expansion, and we know there the mode etc. Just Joern hit a place which wasn't prepared to handle it properly, so either we handle it as you are suggesting by forcing one of the constants into a register, or we simplify the comparison and if it simplifies into a constant, we transform it to something simpler. Jakub
Index: gcc/tree-ssa-ter.c =================================================================== --- gcc/tree-ssa-ter.c (revision 204664) +++ gcc/tree-ssa-ter.c (working copy) @@ -438,6 +439,12 @@ ter_is_replaceable_p (gimple stmt) && !is_gimple_val (gimple_assign_rhs1 (stmt))) return false; + /* Do not propagate "modeless" constants - we may end up confusing the RTL + expanders. Leave the optimization to RTL CCP. */ + if (gimple_assign_single_p (stmt) + && CONSTANT_CLASS_P (gimple_assign_rhs1 (stmt))) + return false; + return true; }