Don't optimize away non-pure/const calls during ccp (PR tree-optimization/51683)

Hi!

For some calls (like memcpy and other builtins that are known to pass
through the first argument) we know the value of the lhs, but still
we shouldn't be replacing the call with just a mere assignment of that
known value to the LHS SSA_NAME, because the call has other side-effects.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk?

2011-12-28  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/51683
	* tree-ssa-propagate.c (substitute_and_fold): Don't optimize away
	calls with side-effects.
	* tree-ssa-ccp.c (ccp_fold_stmt): Likewise.

	* gcc.dg/pr51683.c: New test.


	Jakub

----- Original Message -----
> else if (is_gimple_call (def_stmt))
> {
> + int flags = gimple_call_flags (def_stmt);
> +
> + /* Don't optimize away calls that have side-effects. */
> + if ((flags & (ECF_CONST|ECF_PURE)) == 0
> + || (flags & ECF_LOOPING_CONST_OR_PURE))

This patch does this computation twice; grepping through the tree for ECF_CONST suggests it's done quite a few more times.  Could we get a predicate in gimple.h to encapsulate this?

-Nathan

On Wed, Dec 28, 2011 at 11:53:41AM -0800, Nathan Froyd wrote:
> ----- Original Message -----
> > else if (is_gimple_call (def_stmt))
> > {
> > + int flags = gimple_call_flags (def_stmt);
> > +
> > + /* Don't optimize away calls that have side-effects. */
> > + if ((flags & (ECF_CONST|ECF_PURE)) == 0
> > + || (flags & ECF_LOOPING_CONST_OR_PURE))
> 
> This patch does this computation twice; grepping through the tree for
> ECF_CONST suggests it's done quite a few more times.  Could we get a
> predicate in gimple.h to encapsulate this?

I think it would be an overkill to have a predicate for
nonlooping_const_or_pure_flags, we don't have predicates for similar
RTL or decl flags either.
We write:
        /* We can delete dead const or pure calls as long as they do not
         infinite loop.  */
      && (RTL_CONST_OR_PURE_CALL_P (insn)
          && !RTL_LOOPING_CONST_OR_PURE_CALL_P (insn)))
and not RTL_CONST_OR_PURE_NONLOOPING_CALL_P (insn) etc.

	Jakub

On Wed, Dec 28, 2011 at 8:04 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> For some calls (like memcpy and other builtins that are known to pass
> through the first argument) we know the value of the lhs, but still
> we shouldn't be replacing the call with just a mere assignment of that
> known value to the LHS SSA_NAME, because the call has other side-effects.
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk?

Ok.

Thanks,
Richard.

> 2011-12-28  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/51683
>        * tree-ssa-propagate.c (substitute_and_fold): Don't optimize away
>        calls with side-effects.
>        * tree-ssa-ccp.c (ccp_fold_stmt): Likewise.
>
>        * gcc.dg/pr51683.c: New test.
>
> --- gcc/tree-ssa-propagate.c.jj 2011-11-11 20:54:59.000000000 +0100
> +++ gcc/tree-ssa-propagate.c    2011-12-27 12:23:41.334187258 +0100
> @@ -1056,6 +1056,12 @@ substitute_and_fold (ssa_prop_get_value_
>          }
>        else if (is_gimple_call (def_stmt))
>          {
> +           int flags = gimple_call_flags (def_stmt);
> +
> +           /* Don't optimize away calls that have side-effects.  */
> +           if ((flags & (ECF_CONST|ECF_PURE)) == 0
> +               || (flags & ECF_LOOPING_CONST_OR_PURE))
> +             continue;
>            if (update_call_from_tree (&gsi, val)
>                && maybe_clean_or_replace_eh_stmt (def_stmt, gsi_stmt (gsi)))
>              gimple_purge_dead_eh_edges (gimple_bb (gsi_stmt (gsi)));
> --- gcc/tree-ssa-ccp.c.jj       2011-12-19 09:21:07.000000000 +0100
> +++ gcc/tree-ssa-ccp.c  2011-12-27 12:29:48.620880857 +0100
> @@ -1878,6 +1878,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi
>     case GIMPLE_CALL:
>       {
>        tree lhs = gimple_call_lhs (stmt);
> +       int flags = gimple_call_flags (stmt);
>        tree val;
>        tree argt;
>        bool changed = false;
> @@ -1888,7 +1889,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi
>           type issues.  */
>        if (lhs
>            && TREE_CODE (lhs) == SSA_NAME
> -           && (val = get_constant_value (lhs)))
> +           && (val = get_constant_value (lhs))
> +           /* Don't optimize away calls that have side-effects.  */
> +           && (flags & (ECF_CONST|ECF_PURE)) != 0
> +           && (flags & ECF_LOOPING_CONST_OR_PURE) == 0)
>          {
>            tree new_rhs = unshare_expr (val);
>            bool res;
> --- gcc/testsuite/gcc.dg/pr51683.c.jj   2011-12-27 12:21:43.662925435 +0100
> +++ gcc/testsuite/gcc.dg/pr51683.c      2011-12-27 12:21:23.000000000 +0100
> @@ -0,0 +1,18 @@
> +/* PR tree-optimization/51683 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +static inline void *
> +bar (void *p, void *q, int r)
> +{
> +  return __builtin_memcpy (p, q, r);
> +}
> +
> +void *
> +foo (void *p)
> +{
> +  return bar ((void *) 0x12345000, p, 256);
> +}
> +
> +/* { dg-final { scan-tree-dump "memcpy" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
>
>        Jakub