[Bug c++/102528] New: Unable to inline even trivial coroutines

[Bug c++/102528] New: Unable to inline even trivial coroutines

[redbeard0531 at gmail dot com]

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

            Bug ID: 102528
           Summary: Unable to inline even trivial coroutines
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redbeard0531 at gmail dot com
  Target Milestone: ---

https://godbolt.org/z/aoab9W4xG

This should all compile away, and test() should just be a single ret
instruction. That is not what happens now, even with -O3.

#include <coroutine>

struct simple {
    struct promise_type {
        void return_void() {}
        std::suspend_never initial_suspend() { return {}; }
        std::suspend_never final_suspend() noexcept { return {}; }
        void unhandled_exception() { throw; }
        simple get_return_object() { return {}; }
    };

    std::true_type await_ready() { return {}; }
    void await_suspend(std::coroutine_handle<>) {}
    void await_resume();
};

inline simple test1() {
    co_return;
}

inline simple test2() {
    co_await test1();
    co_return;
}

void test() {
    test2();
}

[Bug c++/102528] Unused out-of-line functions emitted for trivial coroutines

[redbeard0531 at gmail dot com]

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

--- Comment #1 from Mathias Stearn <redbeard0531 at gmail dot com> ---
Sorry, there was a typo in the initial code. I forgot the trivial
implementation of await_resume(). (D'oh!)

Now I can see that test() is just a ret instruction, but there is still a lot
of dead code emitted for the coroutine functions. While it isn't too much for
the trivial functions, for a real use case it would be. And it seems like a bug
anyway  that unused code makes it to the obj file in -O3:
https://godbolt.org/z/bTncofrzd

#include <coroutine>

struct simple {
    struct promise_type {
        void return_void() {}
        std::suspend_never initial_suspend() { return {}; }
        std::suspend_never final_suspend() noexcept { return {}; }
        void unhandled_exception() { throw; }
        simple get_return_object() { return {}; }
    };

    std::true_type await_ready() {return {}; }
    void await_suspend(std::coroutine_handle<>) {}
    void await_resume() {}
};

inline simple test1() {
    co_return;
}

inline simple test2() {
    co_await test1();
    co_return;
}

void test() {
    test2();
}

test1(test1()::_Z5test1v.frame*) [clone .actor]:
        movzx   eax, WORD PTR [rdi+18]
        test    al, 1
        je      .L2
        cmp     ax, 5
        ja      .L3
        mov     edx, 42
        bt      rdx, rax
        jnc     .L3
.L4:
        cmp     BYTE PTR [rdi+17], 0
        jne     .L14
.L1:
        ret
.L2:
        cmp     ax, 2
        je      .L5
        cmp     ax, 4
        je      .L4
        test    ax, ax
        jne     .L3
        mov     QWORD PTR [rdi+24], rdi
.L5:
        cmp     BYTE PTR [rdi+17], 0
        mov     BYTE PTR [rdi+32], 1
        mov     QWORD PTR [rdi], 0
        je      .L1
.L14:
        jmp     operator delete(void*)
test1(test1()::_Z5test1v.frame*) [clone .actor] [clone .cold]:
.L3:
        ud2
test2(test2()::_Z5test2v.frame*) [clone .actor]:
        movzx   eax, WORD PTR [rdi+18]
        test    al, 1
        je      .L16
        cmp     ax, 7
        ja      .L17
        mov     edx, 170
        bt      rdx, rax
        jnc     .L17
.L18:
        cmp     BYTE PTR [rdi+17], 0
        jne     .L33
.L15:
        ret
.L16:
        cmp     ax, 4
        je      .L19
        ja      .L20
        test    ax, ax
        jne     .L34
        mov     QWORD PTR [rdi+24], rdi
.L22:
        mov     BYTE PTR [rdi+32], 1
.L19:
        cmp     BYTE PTR [rdi+17], 0
        mov     QWORD PTR [rdi], 0
        je      .L15
.L33:
        jmp     operator delete(void*)
.L34:
        cmp     ax, 2
        je      .L22
        jmp     .L17
.L20:
        cmp     ax, 6
        je      .L18
        jmp     .L17
test2(test2()::_Z5test2v.frame*) [clone .actor] [clone .cold]:
.L17:
        ud2
test1(test1()::_Z5test1v.frame*) [clone .destroy]:
        movzx   eax, WORD PTR [rdi+18]
        or      eax, 1
        mov     WORD PTR [rdi+18], ax
        cmp     ax, 5
        ja      .L36
        mov     edx, 42
        bt      rdx, rax
        jnc     .L36
        cmp     BYTE PTR [rdi+17], 0
        jne     .L39
        ret
.L39:
        jmp     operator delete(void*)
test1(test1()::_Z5test1v.frame*) [clone .destroy] [clone .cold]:
.L36:
        ud2
test2(test2()::_Z5test2v.frame*) [clone .destroy]:
        movzx   eax, WORD PTR [rdi+18]
        or      eax, 1
        mov     WORD PTR [rdi+18], ax
        cmp     ax, 7
        ja      .L41
        mov     edx, 170
        bt      rdx, rax
        jnc     .L41
        cmp     BYTE PTR [rdi+17], 0
        jne     .L44
        ret
.L44:
        jmp     operator delete(void*)
test2(test2()::_Z5test2v.frame*) [clone .destroy] [clone .cold]:
.L41:
        ud2
test():
        ret

[Bug c++/102528] Unused out-of-line functions emitted for trivial coroutines

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-10-04
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #2 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Mathias Stearn from comment #0)
> https://godbolt.org/z/aoab9W4xG
> 
> This should all compile away, and test() should just be a single ret
> instruction. That is not what happens now, even with -O3.

[Bug c++/102528] Unused out-of-line functions emitted for trivial coroutines

[iains at gcc dot gnu.org]

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

--- Comment #3 from Iain Sandoe <iains at gcc dot gnu.org> ---
 (In reply to Mathias Stearn from comment #0)
 > https://godbolt.org/z/aoab9W4xG
 > 
 > This should all compile away, and test() should just be a single ret
 > instruction. That is not what happens now, even with -O3.

Hi Mathias,

Yes, ideally, that would be the case - so I agree it's a missed optimisation.

On the other hand, to say that the emitted code is unused is not quite right;

We inline the ramp but fail to inline the 'actor' (i.e. the resume).
GCC does not have 'halo' coroutine frame elision, so that we create a frame in
the inlined ramp, and that holds references to the resumer and destroyer.

The net result is that all the code is "used" in the sense that it is referred
to by the emitted test ().

I have an idea (old not new) about altering the internal interfaces in the
generated code to avoid a devirtualisation being necessary to see the inline
opportunity - although I cannot be sure that this is the effect in play, it
seems plausible.

-- so the bottom line is that this is a failure to inline rather than a failure
to elide unused code.

[Bug c++/102528] Unused out-of-line functions emitted for trivial coroutines

[iains at gcc dot gnu.org]

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

--- Comment #4 from Iain Sandoe <iains at gcc dot gnu.org> ---
note I looked at master (trunk) and 11.2.

[Bug c++/102528] Unused out-of-line functions emitted for trivial coroutines

[iains at gcc dot gnu.org]

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

--- Comment #5 from Iain Sandoe <iains at gcc dot gnu.org> ---

hmm let me see if I can reproduce this (I didn't see it in my earlier tests):

.L41:
        ud2
test():
        ret

which would make things look somewhat strange indeed (the functions are all
TU-local - so absent a reference, the 'normal' processes should elide them.

[Bug ipa/102528] Unused out-of-line functions emitted for trivial coroutines

[iains at gcc dot gnu.org]

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

--- Comment #6 from Iain Sandoe <iains at gcc dot gnu.org> ---
#include <coroutine>

struct simple {
    struct promise_type {
        void return_void() {}
        std::suspend_never initial_suspend() { return {}; }
        std::suspend_never final_suspend() noexcept { return {}; }
        void unhandled_exception() { /*throw;*/ }
        simple get_return_object() { return {}; }
    };

    std::true_type await_ready() {return {}; }
    void await_suspend(std::coroutine_handle<>) {}
    void await_resume() {}
};

inline simple
test1() {
    co_return;
}

inline simple
test2() {
    co_await test1();
    co_return;
}

void test() {
    test1();
}

is enough to reproduce at -O2
-std=c++20 -O2 -S -fno-reorder-blocks-and-partition -fno-exceptions
-fno-unwind-tables -fno-asynchronous-unwind-tables 
^^ makes the output a little less complicated.

        .text
        .p2align 4
__Z5test1PZ5test1vE15_Z5test1v.Frame.actor:
<SNIP>
        jmp     __ZdlPv
L3:
        ud2
        .p2align 4
__Z5test1PZ5test1vE15_Z5test1v.Frame.destroy:
        movzwl  32(%rdi), %eax
<SNIP>
        ret
        .p2align 4,,10
        .p2align 3
L19:
        jmp     __ZdlPv
L16:
        ud2
        .p2align 4
        .globl __Z4testv
__Z4testv:
        ret
L21:
        nop
        .ident  "GCC: (GNU) 12.0.0 20211003 (experimental)"

NOTES:
1/ All traces of test2 are correctly elided (so that it seems we need to have
had a use at some earlier point).

2/ at the start of DSE2 we still hold references to the actor and destroyer
functions (the ramp has been inlined, and the ramp function elided).

3/ DSE2 elides the coroutine frame and the references to the two functions, but
it seems that is too late in the pipeline for them to be removed?

[Bug ipa/102528] Unused out-of-line functions emitted for trivial coroutines

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
For early opts the issue is that modref somehow gives up and we are not able
to DSE the stores in test2 doing

  <bb 2> :
  _9 = operator new (40);
  _9->_Coro_frame_needs_freeD.9018 = 1;
  _9->_Coro_resume_fnD.9013 = test2D.8994;
  _9->_Coro_destroy_fnD.9014 = test2D.8997;
  _9->_Coro_resume_indexD.9017 = 0;
  test2 (_9);

(note all calls to 'test2' dump this way even with -uid, but they are to
different clones of 'test2' ...).  Make sure to use the testcase from comment#1

[Bug ipa/102528] Unused out-of-line functions emitted for trivial coroutines

[redbeard0531 at gmail dot com]

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

--- Comment #8 from Mathias Stearn <redbeard0531 at gmail dot com> ---
Sorry again about the confusion caused by my typo. I am not able to edit the
comment to make it clear that the comment#0 should be ignored. If that happens
again, would it be better for me to close the ticket and create a new one?

[Bug ipa/102528] Unused out-of-line functions emitted for trivial coroutines

[iains at gcc dot gnu.org]

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

--- Comment #9 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Mathias Stearn from comment #8)
> Sorry again about the confusion caused by my typo. I am not able to edit the
> comment to make it clear that the comment#0 should be ignored. If that
> happens again, would it be better for me to close the ticket and create a
> new one?

hmm ... I thought that adding the tag "obsolete" should hide it - but maybe it
has to be done by the OP. I tried to add it and it didn't seem to make any
difference.