[Bug c++/96780] New: debuginfo for std::move and std::forward isn't useful

[Bug c++/96780] New: debuginfo for std::move and std::forward isn't useful

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

            Bug ID: 96780
           Summary: debuginfo for std::move and std::forward isn't useful
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redi at gcc dot gnu.org
  Target Milestone: ---

When debugging I don't want to step into std::move and std::forward, ever.
They're effectively just casts, and although g++ correctly optimises them away
to nothing, it leaves debuginfo that makes gdb step into them.

Is there any good reason to ever step into them, or could we special case them
in g++ to not emit any debuginfo, at least when inlined?

I tried adding __attribute__((__always_inline__,__artificial__)) but that
seemed to make it even worse.

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-08-25
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
                 CC|                            |mpolacek at gcc dot gnu.org

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ppalka at gcc dot gnu.org

--- Comment #1 from Patrick Palka <ppalka at gcc dot gnu.org> ---
I wonder if

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #2 from Patrick Palka <ppalka at gcc dot gnu.org> ---
I wonder if it would be worthwhile to fold calls to std::move and std::forward
altogether in the frontend.

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I think that would be great.

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #4 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Created attachment 51732
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51732&action=edit
rough patch that folds calls to std::move/forward

Does the attached rough patch help?

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yes, from a very quick test, it does exactly what I want.

As discussed on IRC, we might not want to do this folding at -O0 (although I'd
personally be happy with it unconditionally).

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |ppalka at gcc dot gnu.org

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Please declare those functions in cp-tree.h though.

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[vittorio.romeo at outlook dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #7 from Vittorio Romeo <vittorio.romeo at outlook dot com> ---
> As discussed on IRC, we might not want to do this folding at -O0 (although I'd personally be happy with it unconditionally).

I think you should reconsider this as discussed in these places:
- https://github.com/llvm/llvm-project/issues/53689
- https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104719

Compiling in `-O0` is a valid choice when trying to maximize compilation speed
and debuggability, yet pretty much everyone seems to agree that they'd like to
never see `std::move`/`std::forward` in their debugger nor have them introduce
any performance overhead, even in `-O0`.

I would also suggest, as an extension, to consider a more general approach for
other standard library functions. As an example, there are good gains to be
made in terms of debug performance for things like `std::unique_ptr` (see
https://github.com/llvm/llvm-project/issues/53689#issuecomment-1055669228) or
`std::vector<T>::iterator`.

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
But -O0 often compiles slower, because so much more code gets emitted and must
be assembled and linked.

So -O1 or -Og is often better for all of debugging and compilation speed and
runtime performance.

That aside, I don't feel strongly about whether this is done at -O0. Somebody
will want the other choice whatever we do.

The unique_ptr discussion seems off topic here, let's keep the more general
discussion to PR 104719 please.

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
As well as folding move and forward, it probably makes sense to do the same for
as_const and addressof (and our internal __addressof). addressof is
particularly annoying because it's uglier *and* slower than taking the address,
but 100% necessary because of ADL.

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org

--- Comment #10 from Jason Merrill <jason at gcc dot gnu.org> ---
In gdb nowadays, if you never want to step into a function, you can tell gdb
that with the 'skip' command.  There are a bunch of those in the gcc .gdbinit
for tiny functions like std::move, and I have a few in my ~/.gdbinit as well.

It seems the libstdc++ python hooks could set that up for users with
gdb.execute ("skip std::move")?

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #11 from Jason Merrill <jason at gcc dot gnu.org> ---
I'm surprised that adding the "artificial" attribute didn't work; I thought the
main point of that attribute was to automatically skip the function in the
debugger/profiler.  I guess that never got implemented in gdb?

Related threads about avoiding debugging of uninteresting functions, which led
to the addition of the "artificial" attribute:

https://gcc.gnu.org/legacy-ml/gcc-patches/2005-07/threads.html#01969
https://gcc.gnu.org/legacy-ml/gcc-patches/2007-08/threads.html#02300

But these functions aren't really artificial, just tiny, so using that
attribute seems wrong.

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #12 from Patrick Palka <ppalka at gcc dot gnu.org> ---
I should mention I noticed a significant reduction in compile time, memory
usage and unstripped object file size in some cases with the proposed patch at
https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591091.html.

For instance, for the range-v3 test file
test/algorithm/set_symmetric_difference4.cpp compiled with -O2 -g and a
non-checking compiler, compile time/GC allocations/object file size decreases
from
  8.29s/551M/3508K
to
  8.00s/541M/3104K.

The object file size decrease (naively measured with du) here is particularly
surprising, nearly a 12% decrease.  I suppose this means debug info for inlined
calls to std::move/forward accounts for >=12% of the debug info size, which we
no longer emit with the patch.

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jason Merrill from comment #11)
> I'm surprised that adding the "artificial" attribute didn't work; I thought
> the main point of that attribute was to automatically skip the function in
> the debugger/profiler.  I guess that never got implemented in gdb?

It seems to work with optimization, but not at -O0:

$ gdb -q -ex start -ex step -ex step -ex cont -ex q a.out
Reading symbols from a.out...
Temporary breakpoint 1 at 0x40110e: file move.C, line 16.
Starting program: /tmp/a.out 

Temporary breakpoint 1, main () at move.C:16
16        int i = 0;
17        return std::move(i);
std::move<int&> (__t=@0x7fffffffd77c: 0) at move.C:11
11          { return static_cast<typename
std::remove_reference<_Tp>::type&&>(__t); }
Continuing.
[Inferior 1 (process 1629725) exited normally]

I think I must have only tested -O0 last time.


> But these functions aren't really artificial, just tiny, so using that
> attribute seems wrong.

I think there's a case to be made for things like std::__addressof and
std::__is_constant_evaluated being "artificial" because they are just wrappers
around a built-in, and purely impl details. But I agree that std::move and
std::forward are not "artificial" in the sense of the DW_AT_artificial tag:
https://dwarfstd.org/doc/DWARF5.pdf#page=65

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jason Merrill from comment #10)
> It seems the libstdc++ python hooks could set that up for users with
> gdb.execute ("skip std::move")?

No, that doesn't work. You need to use 'skip "std::move<int&>"' otherwise it
doesn't skip it:

$ gdb -q -ex start -ex 'skip std::move' -ex step -ex step -ex cont -ex q a.out
Reading symbols from a.out...
Temporary breakpoint 1 at 0x40110e: file move.C, line 16.
Starting program: /tmp/a.out 

Temporary breakpoint 1, main () at move.C:16
16        int i = 0;
Function std::move will be skipped when stepping.
17        return std::move(i);
std::move<int&> (__t=@0x7fffffffd77c: 0) at move.C:11
11          { return static_cast<typename
std::remove_reference<_Tp>::type&&>(__t); }


And std::move* doesn't work either:


$ gdb -q -ex start -ex 'skip std::move*' -ex step -ex step -ex cont -ex q a.out
Reading symbols from a.out...
Temporary breakpoint 1 at 0x40110e: file move.C, line 16.
Starting program: /tmp/a.out 

Temporary breakpoint 1, main () at move.C:16
16        int i = 0;
Function std::move* will be skipped when stepping.
17        return std::move(i);
std::move<int&> (__t=@0x7fffffffd77c: 0) at move.C:11
11          { return static_cast<typename
std::remove_reference<_Tp>::type&&>(__t); }

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #15 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #14)
From
https://sourceware.org/gdb/current/onlinedocs/gdb/Skipping-Over-Functions-and-Files.html#Skipping-Over-Functions-and-Files

it looks like you want skip -rfu; it gives the example

skip -rfu ^std::(allocator|basic_string)<.*>::~?\1 *\(

to skip string constructors and destructors.

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:e55c5e24b97ad8ddc44588da18e894c139e02c0a

commit r12-7668-ge55c5e24b97ad8ddc44588da18e894c139e02c0a
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Mar 16 08:25:54 2022 -0400

    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,
    which persists even after the call gets inlined, for an operation that's
    never interesting to debug.

    This patch addresses this problem 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.  This behavior is controlled by a
    new flag -ffold-simple-inlines, and otherwise by -fno-inline, so that
    users can enable this folding with -O0 (which implies -fno-inline).

    After this patch with -O2 and a non-checking compiler, debug info size
    for some testcases from range-v3 and cmcstl2 decreases by as much as ~10%
    and overall compile time and memory usage decreases by ~2%.

            PR c++/96780

    gcc/ChangeLog:

            * doc/invoke.texi (C++ Dialect Options): Document
            -ffold-simple-inlines.

    gcc/c-family/ChangeLog:

            * c.opt: Add -ffold-simple-inlines.

    gcc/cp/ChangeLog:

            * cp-gimplify.cc (cp_fold) <case CALL_EXPR>: Fold calls to
            std::move/forward and other cast-like functions into simple
            casts.

    gcc/testsuite/ChangeLog:

            * g++.dg/opt/pr96780.C: New test.

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

--- Comment #17 from Jason Merrill <jason at gcc dot gnu.org> ---
*** Bug 104719 has been marked as a duplicate of this bug. ***

[Bug c++/96780] debuginfo for std::move and std::forward isn't useful

[moncef.mechri at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96780

Moncef Mechri <moncef.mechri at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |moncef.mechri at gmail dot com

--- Comment #18 from Moncef Mechri <moncef.mechri at gmail dot com> ---
Currently, -ffold-simple-inlines is disabled when optimizations are disabled.

Since it is pretty much standard practice to disable optimizations in debug
builds (yes, I am aware that -Og exists), perhaps it would be a good idea to
make -ffold-simple-inlines opt-out instead of opt-in even for non-optimized
builds?