Re: [PATCH] c++: fold calls to std::move/forward [PR96780]

[Jason Merrill <jason@redhat.com>]

On 3/1/22 18:08, Patrick Palka wrote:
> A well-formed call to std::move/forward is equivalent to a cast, but the
> former being a function call means it comes with bloated debug info, which
> persists even after the call has been inlined away, for an operation that
> is never interesting to debug.
> 
> This patch addresses this problem in a relatively ad-hoc way by folding
> calls to std::move/forward into casts as part of the frontend's general
> expression folding routine.  After this patch with -O2 and a non-checking
> compiler, debug info size for some testcases decreases by about ~10% and
> overall compile time and memory usage decreases by ~2%.

Impressive.  Which testcases?

Do you also want to handle addressof and as_const in this patch, as 
Jonathan suggested?

I think we can do this now, and think about generalizing more in stage 1.

> Bootstrapped and regtested on x86_64-pc-linux-gnu, is this something we
> want to consider for GCC 12?
> 
> 	PR c++/96780
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-gimplify.cc (cp_fold) <case CALL_EXPR>: When optimizing,
> 	fold calls to std::move/forward into simple casts.
> 	* cp-tree.h (is_std_move_p, is_std_forward_p): Declare.
> 	* typeck.cc (is_std_move_p, is_std_forward_p): Export.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/opt/pr96780.C: New test.
> ---
>   gcc/cp/cp-gimplify.cc              | 18 ++++++++++++++++++
>   gcc/cp/cp-tree.h                   |  2 ++
>   gcc/cp/typeck.cc                   |  6 ++----
>   gcc/testsuite/g++.dg/opt/pr96780.C | 24 ++++++++++++++++++++++++
>   4 files changed, 46 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/opt/pr96780.C
> 
> diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
> index d7323fb5c09..0b009b631c7 100644
> --- a/gcc/cp/cp-gimplify.cc
> +++ b/gcc/cp/cp-gimplify.cc
> @@ -2756,6 +2756,24 @@ cp_fold (tree x)
>   
>       case CALL_EXPR:
>         {
> +	if (optimize

I think this should check flag_no_inline rather than optimize.

> +	    && (is_std_move_p (x) || is_std_forward_p (x)))
> +	  {
> +	    /* When optimizing, "inline" calls to std::move/forward by
> +	       simply folding them into the corresponding cast.  This is
> +	       cheaper than relying on the inliner to do so, and also
> +	       means we avoid generating useless debug info for them at all.
> +
> +	       At this point the argument has already been coerced into a
> +	       reference, so it suffices to use a NOP_EXPR to express the
> +	       reference-to-reference cast.  */
> +	    r = CALL_EXPR_ARG (x, 0);
> +	    if (!same_type_p (TREE_TYPE (x), TREE_TYPE (r)))
> +	      r = build_nop (TREE_TYPE (x), r);
> +	    x = cp_fold (r);
> +	    break;
> +	  }
> +
>   	int sv = optimize, nw = sv;
>   	tree callee = get_callee_fndecl (x);
>   
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 37d462fca6e..ab828730b03 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -8089,6 +8089,8 @@ extern tree finish_right_unary_fold_expr     (tree, int);
>   extern tree finish_binary_fold_expr          (tree, tree, int);
>   extern tree treat_lvalue_as_rvalue_p	     (tree, bool);
>   extern bool decl_in_std_namespace_p	     (tree);
> +extern bool is_std_move_p		     (tree);
> +extern bool is_std_forward_p		     (tree);
>   
>   /* in typeck2.cc */
>   extern void require_complete_eh_spec_types	(tree, tree);
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index bddc83759ad..a3644f8e7f7 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -62,8 +62,6 @@ static bool maybe_warn_about_returning_address_of_local (tree, location_t = UNKN
>   static void error_args_num (location_t, tree, bool);
>   static int convert_arguments (tree, vec<tree, va_gc> **, tree, int,
>                                 tsubst_flags_t);
> -static bool is_std_move_p (tree);
> -static bool is_std_forward_p (tree);
>   
>   /* Do `exp = require_complete_type (exp);' to make sure exp
>      does not have an incomplete type.  (That includes void types.)
> @@ -10207,7 +10205,7 @@ decl_in_std_namespace_p (tree decl)
>   
>   /* Returns true if FN, a CALL_EXPR, is a call to std::forward.  */
>   
> -static bool
> +bool
>   is_std_forward_p (tree fn)
>   {
>     /* std::forward only takes one argument.  */
> @@ -10224,7 +10222,7 @@ is_std_forward_p (tree fn)
>   
>   /* Returns true if FN, a CALL_EXPR, is a call to std::move.  */
>   
> -static bool
> +bool
>   is_std_move_p (tree fn)
>   {
>     /* std::move only takes one argument.  */
> diff --git a/gcc/testsuite/g++.dg/opt/pr96780.C b/gcc/testsuite/g++.dg/opt/pr96780.C
> new file mode 100644
> index 00000000000..ca24b2802bb
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/opt/pr96780.C
> @@ -0,0 +1,24 @@
> +// PR c++/96780
> +// Verify calls to std::move/forward are folded away by the frontend.
> +// { dg-do compile { target c++11 } }
> +// { dg-additional-options "-O -fdump-tree-gimple" }
> +
> +#include <utility>
> +
> +struct A;
> +
> +extern A& a;
> +extern const A& ca;
> +
> +void f() {
> +  auto&& x1 = std::move(a);
> +  auto&& x2 = std::forward<A>(a);
> +  auto&& x3 = std::forward<A&>(a);
> +
> +  auto&& x4 = std::move(ca);
> +  auto&& x5 = std::forward<const A>(ca);
> +  auto&& x6 = std::forward<const A&>(ca);
> +}
> +
> +// { dg-final { scan-tree-dump-not "= std::move" "gimple" } }
> +// { dg-final { scan-tree-dump-not "= std::forward" "gimple" } }