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

[Jason Merrill <jason@redhat.com>]

On 3/10/22 11:27, Patrick Palka wrote:
> On Wed, 9 Mar 2022, Jason Merrill wrote:
> 
>> 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?
> 
> I saw the largest percent reductions in debug file object size in
> various tests from cmcstl2 and range-v3, e.g.
> test/algorithm/set_symmetric_difference4.cpp and .../rotate_copy.cpp
> (which are among their biggest tests).
> 
> Significant reductions in debug object file size can be observed in
> some libstdc++ testcases too, such as a 5.5% reduction in
> std/ranges/adaptor/join.cc
> 
>>
>> Do you also want to handle addressof and as_const in this patch, as Jonathan
>> suggested?
> 
> Yes, good idea.  Since each of their argument and return types are
> indirect types, I think we can use the same NOP_EXPR-based folding for
> them.
> 
>>
>> 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.
> 
> Sounds good.
> 
> Here's a patch that extends the folding to as_const and addressof (as
> well as __addressof, which I'm kind of unsure about since it's
> non-standard).  I suppose it also doesn't hurt to verify that the return
> and argument type of the function are sane before we commit to folding.
> 
> -- >8 --
> 
> Subject: [PATCH] c++: fold calls to std::move/forward [PR96780]
> 
> A well-formed call to std::move/forward is equivalent to a cast, but the
> former being a function call means the compiler generates debug info for
> it, which persists even after the call has been inlined away, for an
> operation that's never interesting to debug.
> 
> This patch addresses this problem in a relatively ad-hoc way by folding
> calls to std::move/forward and other cast-like functions into simple
> 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%.
> 
> 	PR c++/96780
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-gimplify.cc (cp_fold) <case CALL_EXPR>: When optimizing,
> 	fold calls to std::move/forward and other cast-like functions
> 	into simple casts.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/opt/pr96780.C: New test.
> ---
>   gcc/cp/cp-gimplify.cc              | 36 +++++++++++++++++++++++++++-
>   gcc/testsuite/g++.dg/opt/pr96780.C | 38 ++++++++++++++++++++++++++++++
>   2 files changed, 73 insertions(+), 1 deletion(-)
>   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..efc4c8f0eb9 100644
> --- a/gcc/cp/cp-gimplify.cc
> +++ b/gcc/cp/cp-gimplify.cc
> @@ -2756,9 +2756,43 @@ cp_fold (tree x)
>   
>       case CALL_EXPR:
>         {
> -	int sv = optimize, nw = sv;
>   	tree callee = get_callee_fndecl (x);
>   
> +	/* "Inline" calls to std::move/forward and other cast-like functions
> +	   by simply folding them into the corresponding cast determined by
> +	   their return type.  This is cheaper than relying on the middle-end
> +	   to do so, and also means we avoid generating useless debug info for
> +	   them at all.
> +
> +	   At this point the argument has already been converted into a
> +	   reference, so it suffices to use a NOP_EXPR to express the
> +	   cast.  */
> +	if (!flag_no_inline

In our conversation yesterday it occurred to me that we might make this 
a separate flag that defaults to the value of flag_no_inline; I was 
thinking of -ffold-simple-inlines.  Then Vittorio et al can specify that 
explicitly at -O0 if they'd like.

> +	    && call_expr_nargs (x) == 1
> +	    && decl_in_std_namespace_p (callee)
> +	    && DECL_NAME (callee) != NULL_TREE
> +	    && (id_equal (DECL_NAME (callee), "move")
> +		|| id_equal (DECL_NAME (callee), "forward")
> +		|| id_equal (DECL_NAME (callee), "addressof")
> +		/* This addressof equivalent is used in libstdc++.  */
> +		|| id_equal (DECL_NAME (callee), "__addressof")
> +		|| id_equal (DECL_NAME (callee), "as_const")))
> +	  {
> +	    r = CALL_EXPR_ARG (x, 0);
> +	    /* Check that the return and arguments types are sane before
> +	       folding.  */
> +	    if (INDIRECT_TYPE_P (TREE_TYPE (x))
> +		&& INDIRECT_TYPE_P (TREE_TYPE (r)))
> +	      {
> +		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;
> +
>   	/* Some built-in function calls will be evaluated at compile-time in
>   	   fold ().  Set optimize to 1 when folding __builtin_constant_p inside
>   	   a constexpr function so that fold_builtin_1 doesn't fold it to 0.  */
> diff --git a/gcc/testsuite/g++.dg/opt/pr96780.C b/gcc/testsuite/g++.dg/opt/pr96780.C
> new file mode 100644
> index 00000000000..1a426b1328b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/opt/pr96780.C
> @@ -0,0 +1,38 @@
> +// 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);
> +
> +  auto x7 = std::addressof(a);
> +  auto x8 = std::addressof(ca);
> +#if __GLIBCXX__
> +  auto x9 = std::__addressof(a);
> +  auto x10 = std::__addressof(ca);
> +#endif
> +#if __cpp_lib_as_const
> +  auto&& x11 = std::as_const(a);
> +  auto&& x12 = std::as_const(ca);
> +#endif
> +}
> +
> +// { dg-final { scan-tree-dump-not "= std::move" "gimple" } }
> +// { dg-final { scan-tree-dump-not "= std::forward" "gimple" } }
> +// { dg-final { scan-tree-dump-not "= std::addressof" "gimple" } }
> +// { dg-final { scan-tree-dump-not "= std::__addressof" "gimple" } }
> +// { dg-final { scan-tree-dump-not "= std::as_const" "gimple" } }