Re: [PATCH] Avoid sinking BLKmode stores in functions with local explicit register vars (PR target/39139)

[Richard Guenther <richard dot guenther at gmail dot com>]

On Tue, Feb 10, 2009 at 4:27 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> BLKmode stores (either aggregate copies or = {} clearing) often involves
> either a function call or on several architectures specialized stringop insns
> that have specific hard req requirements.  Thus it is unsafe to sink these
> into a live range of some local explicit register variable.  The following
> patch just disables sinking BLKmode stores altogether in functions that
> contain any such variables, I think they are rare enough that this shouldn't
> have noticeable performance impact.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2009-02-10  Jakub Jelinek  <jakub@redhat.com>
>
>        PR target/39139
>        * function.h (struct function): Add has_local_explicit_reg_vars
>        bit.
>        * gimplify.c (gimplify_bind_expr): Set it if local DECL_HARD_REGISTER
>        VAR_DECLs were seen.
>        * tree-ssa-live.c (remove_unused_locals): Recompute
>        cfun->has_local_explicit_reg_vars.
>        * tree-ssa-sink.c (statement_sink_location): Don't sink BLKmode
>        copies or clearings if cfun->has_local_explicit_reg_vars.
>
>        * gcc.target/i386/pr39139.c: New test.
>
> --- gcc/function.h.jj   2009-01-13 18:32:17.000000000 +0100
> +++ gcc/function.h      2009-02-10 10:25:42.000000000 +0100
> @@ -1,6 +1,6 @@
>  /* Structure for saving state for a nested function.
>    Copyright (C) 1989, 1992, 1993, 1994, 1995, 1996, 1997, 1998,
> -   1999, 2000, 2003, 2004, 2005, 2006, 2007, 2008
> +   1999, 2000, 2003, 2004, 2005, 2006, 2007, 2008, 2009
>    Free Software Foundation, Inc.
>
>  This file is part of GCC.
> @@ -596,6 +596,10 @@ struct function GTY(())
>
>   /* Nonzero if pass_tree_profile was run on this function.  */
>   unsigned int after_tree_profile : 1;
> +
> +  /* Nonzero if this function has local DECL_HARD_REGISTER variables.
> +     In this case code motion has to be done more carefully.  */
> +  unsigned int has_local_explicit_reg_vars : 1;
>  };
>
>  /* If va_list_[gf]pr_size is set to this, it means we don't know how
> --- gcc/gimplify.c.jj   2009-01-26 15:24:44.000000000 +0100
> +++ gcc/gimplify.c      2009-02-10 10:29:22.000000000 +0100
> @@ -1,6 +1,6 @@
>  /* Tree lowering pass.  This pass converts the GENERIC functions-as-trees
>    tree representation into the GIMPLE form.
> -   Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008
> +   Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009
>    Free Software Foundation, Inc.
>    Major work done by Sebastian Pop <s.pop@laposte.net>,
>    Diego Novillo <dnovillo@redhat.com> and Jason Merrill <jason@redhat.com>.
> @@ -1240,6 +1240,9 @@ gimplify_bind_expr (tree *expr_p, gimple
>            omp_add_variable (gimplify_omp_ctxp, t, GOVD_LOCAL | GOVD_SEEN);
>
>          DECL_SEEN_IN_BIND_EXPR_P (t) = 1;
> +
> +         if (DECL_HARD_REGISTER (t) && !is_global_var (t) && cfun)
> +           cfun->has_local_explicit_reg_vars = true;
>        }
>
>       /* Preliminarily mark non-addressed complex variables as eligible
> --- gcc/tree-ssa-live.c.jj      2008-10-23 13:21:40.000000000 +0200
> +++ gcc/tree-ssa-live.c 2009-02-10 10:23:13.000000000 +0100
> @@ -1,5 +1,5 @@
>  /* Liveness for SSA trees.
> -   Copyright (C) 2003, 2004, 2005, 2007, 2008 Free Software Foundation,
> +   Copyright (C) 2003, 2004, 2005, 2007, 2008, 2009 Free Software Foundation,
>    Inc.
>    Contributed by Andrew MacLeod <amacleod@redhat.com>
>
> @@ -642,6 +642,8 @@ remove_unused_locals (void)
>          TREE_USED (e->goto_block) = true;
>     }
>
> +  cfun->has_local_explicit_reg_vars = false;
> +
>   /* Remove unmarked local vars from local_decls.  */
>   for (cell = &cfun->local_decls; *cell; )
>     {
> @@ -663,6 +665,10 @@ remove_unused_locals (void)
>              continue;
>            }
>        }
> +      else if (TREE_CODE (var) == VAR_DECL
> +              && DECL_HARD_REGISTER (var)
> +              && !is_global_var (var))
> +       cfun->has_local_explicit_reg_vars = true;
>       cell = &TREE_CHAIN (*cell);
>     }
>
> --- gcc/tree-ssa-sink.c.jj      2008-10-23 13:21:39.000000000 +0200
> +++ gcc/tree-ssa-sink.c 2009-02-10 11:00:05.000000000 +0100
> @@ -1,5 +1,6 @@
>  /* Code sinking for trees
> -   Copyright (C) 2001, 2002, 2003, 2004, 2007 Free Software Foundation, Inc.
> +   Copyright (C) 2001, 2002, 2003, 2004, 2007, 2009
> +   Free Software Foundation, Inc.
>    Contributed by Daniel Berlin <dan@dberlin.org>
>
>  This file is part of GCC.
> @@ -301,7 +302,12 @@ statement_sink_location (gimple stmt, ba
>      We can't sink statements that have volatile operands.
>
>      We don't want to sink dead code, so anything with 0 immediate uses is not
> -     sunk.
> +     sunk.
> +
> +     Don't sink BLKmode assignments if current function has any local explicit
> +     register variables, as BLKmode assignments may involve memcpy or memset
> +     calls or, on some targets, inline expansion thereof that sometimes need
> +     to use specific hard registers.
>
>   */
>   code = gimple_assign_rhs_code (stmt);
> @@ -311,7 +317,9 @@ statement_sink_location (gimple stmt, ba
>       || code == FILTER_EXPR
>       || is_hidden_global_store (stmt)
>       || gimple_has_volatile_ops (stmt)
> -      || !ZERO_SSA_OPERANDS (stmt, SSA_OP_VUSE))
> +      || !ZERO_SSA_OPERANDS (stmt, SSA_OP_VUSE)
> +      || (cfun->has_local_explicit_reg_vars
> +         && TYPE_MODE (TREE_TYPE (gimple_assign_lhs (stmt))) == BLKmode))
>     return false;
>
>   FOR_EACH_SSA_DEF_OPERAND (def_p, stmt, iter, SSA_OP_ALL_DEFS)
> --- gcc/testsuite/gcc.target/i386/pr39139.c.jj  2009-02-10 11:08:22.000000000 +0100
> +++ gcc/testsuite/gcc.target/i386/pr39139.c     2009-02-10 11:07:28.000000000 +0100
> @@ -0,0 +1,39 @@
> +/* PR target/39139 */
> +/* { dg-do compile } */
> +/* { dg-options "-Os" } */
> +
> +#ifdef __x86_64__
> +# define AX_REG asm ("rax")
> +# define DI_REG asm ("rdi")
> +# define SI_REG asm ("rsi")
> +#else
> +# define AX_REG asm ("eax")
> +# define DI_REG asm ("edi")
> +# define SI_REG asm ("esi")
> +#endif
> +
> +static inline int
> +foo (unsigned int x, void *y)
> +{
> +  register unsigned long r AX_REG;
> +  register unsigned long a1 DI_REG;
> +  register unsigned long a2 SI_REG;
> +  a1 = (unsigned long) x;
> +  a2 = (unsigned long) y;
> +  asm volatile ("" : "=r" (r), "+r" (a1), "+r" (a2) : : "memory");
> +  return (int) r;
> +}
> +
> +struct T { unsigned long t1, t2; unsigned int t3, t4, t5; };
> +
> +int
> +bar (unsigned long x, unsigned int y, unsigned long u, unsigned int v)
> +{
> +  long r;
> +  struct T e = { .t1 = x, .t2 = u };
> +
> +  if (x << y != u << v)
> +    return 5;
> +  r = foo (11, &e);
> +  return e.t3 == x;
> +}
>
>        Jakub
>