On Thu, 20 Dec 2012, Richard Biener wrote:
> On Thu, 20 Dec 2012, Jakub Jelinek wrote:
>
> > On Thu, Dec 20, 2012 at 02:51:55PM +0100, Richard Biener wrote:
> > > In the PR we perform expression replacement of an FP operation
> > > across a builtin call that sets the FP control register. This
> > > patch restricts replacement across calls further, from allowing
> > > all builtins to only allowing those without side-effects.
> > >
> > > Allowing replacement over calls at all was to not pessimize
> > > FP code generation for example for sqrt which is most often
> > > expanded to a single instruction.
> > >
> > > Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> > >
> > > Comments?
> >
> > Wouldn't it be better to have there a list of known builtins over which it
> > is fine to do TER? I'd bet most of memory or string builtins that don't
> > call malloc/free should be still ok, but they surely have side-effects.
>
> I'm not sure - the original reason was that replacing across calls
> made us spill more because there was a call. We agreed that replacing
> across calls isn't usually a good idea but put in the (admittedly bad)
> workaround to still allow doing so across likely-not-calls.
> string builtins generally will expand to calls though.
>
> I was thinking of even making it stronger and increment "cur_call_cnt"
> when the stmt (even non-call) has side-effects (would for example
> cover volatile asms or general volatile touching insns).
Like so:
Index: gcc/tree-ssa-ter.c
===================================================================
--- gcc/tree-ssa-ter.c (revision 194632)
+++ gcc/tree-ssa-ter.c (working copy)
@@ -681,12 +681,13 @@ find_replaceable_in_bb (temp_expr_table_
kill_expr (tab, partition);
}
- /* Increment counter if this is a non BUILT_IN call. We allow
- replacement over BUILT_IN calls since many will expand to inline
- insns instead of a true call. */
- if (is_gimple_call (stmt)
- && !((fndecl = gimple_call_fndecl (stmt))
- && DECL_BUILT_IN (fndecl)))
+ /* Increment counter if this is not a BUILT_IN call or a stmt with
+ side-effects. We allow replacement over BUILT_IN calls
+ since many will expand to inline insns instead of a true call.
*/
+ if (gimple_has_side_effects (stmt)
+ || (is_gimple_call (stmt)
+ && !((fndecl = gimple_call_fndecl (stmt))
+ && DECL_BUILT_IN (fndecl))))
cur_call_cnt++;
/* Now see if we are creating a new expression or not. */
Richard.
