Skip to content

clang optimization issue in coroutines(not allocations) #64933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kelbon opened this issue Aug 23, 2023 · 2 comments
Closed

clang optimization issue in coroutines(not allocations) #64933

kelbon opened this issue Aug 23, 2023 · 2 comments
Labels

Comments

@kelbon
Copy link
Contributor

kelbon commented Aug 23, 2023

https://godbolt.org/z/5hTE69j1b

starting row 82 in asm there are something strange in 'trunk', im 99% sure someone apply 'cold' attribute or 'never inline' to all functions in final suspend or smth like.

All calls must be inlined, its literaly one-row functions.

g1() [clone .resume]:                          # @g1() [clone .resume]
        push    rbx
        sub     rsp, 16
        cmp     byte ptr [rdi + 44], 0
        je      .LBB8_1
        lea     rbx, [rdi + 16]
        mov     qword ptr [rdi], 0
        call    std::__n4861::coroutine_handle<dd::generator_promise<int> >::from_address(void*)
        mov     qword ptr [rsp + 8], rax
        lea     rdi, [rsp + 8]
        call    std::__n4861::coroutine_handle<dd::generator_promise<int> >::operator std::__n4861::coroutine_handle<void>() const
        mov     rdi, rbx
        mov     rsi, rax
        call    dd::generator_promise<int>::await_suspend(std::__n4861::coroutine_handle<void>) const
        mov     qword ptr [rsp], rax
        mov     rdi, rsp
        call    std::__n4861::coroutine_handle<void>::address() const
        mov     rdi, rax
        add     rsp, 16
        pop     rbx
        jmp     qword ptr [rax]                 # TAILCALL
.LBB8_1:
@llvmbot
Copy link
Member

llvmbot commented Aug 23, 2023

@llvm/issue-subscribers-coroutines

@ChuanqiXu9
Copy link
Member

ChuanqiXu9 commented Aug 24, 2023

Sorry this is a known regression after c467245. And the proposed solution may solve the issue: #64945. Then let's track the issue there.

ChuanqiXu9 added a commit that referenced this issue Aug 24, 2023
… regressions

The fix we sent for #56301
may bring performance regressions. But we didn't mention it in the
ReleaseNotes so that users may get confused. e.g,
#64933. So this patch
mentions the possible side effect and the potential solutions in
#64945 to avoid
misunderstandings.
llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 25, 2023
… regressions

The fix we sent for llvm/llvm-project#56301
may bring performance regressions. But we didn't mention it in the
ReleaseNotes so that users may get confused. e.g,
llvm/llvm-project#64933. So this patch
mentions the possible side effect and the potential solutions in
llvm/llvm-project#64945 to avoid
misunderstandings.
tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 27, 2023
… regressions

The fix we sent for llvm/llvm-project#56301
may bring performance regressions. But we didn't mention it in the
ReleaseNotes so that users may get confused. e.g,
llvm/llvm-project#64933. So this patch
mentions the possible side effect and the potential solutions in
llvm/llvm-project#64945 to avoid
misunderstandings.
ChuanqiXu9 added a commit that referenced this issue Aug 28, 2023
…ecified as always_inline already

Address #64933 and partially
#64945.

After c467245, we will add a noinline attribute to the await_suspend
member function of an awaiter if the awaiter has any non static member
functions.

Obviously, this decision will bring some performance regressions. And
people may complain about this while the long term solution may not be
available soon. In such cases, it is better to provide a solution for
the users who met the regression surprisingly.

Also it is natural to not prevent the inlining if the function is marked
as always_inline by the users already.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants