Skip to content

clang 18 fails to apply HALO #94215

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
DarkWingMcQuack opened this issue Jun 3, 2024 · 7 comments
Closed

clang 18 fails to apply HALO #94215

DarkWingMcQuack opened this issue Jun 3, 2024 · 7 comments
Assignees
Labels
clang Clang issues not falling into any other category coroutines C++20 coroutines question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead!

Comments

@DarkWingMcQuack
Copy link

While clang 17.0.1 is able to elide the heap allocations of the coroutines, clang 18.1.0 does not seem to be able to do so even for simple coroutine code like this:

struct initiation : public std::suspend_always
{
    struct promise_type
    {
        constexpr auto initial_suspend() const noexcept { return std::suspend_always{}; }

        constexpr auto final_suspend() const noexcept {
            struct final_awaiter : public std::suspend_always
            {
                auto await_suspend(std::coroutine_handle<initiation::promise_type> h) noexcept -> std::coroutine_handle<>
                {
                    return h.promise().previous;
                }
            };
            return final_awaiter{};
        }

        [[noreturn]] void unhandled_exception() const { std::terminate(); }

        constexpr auto get_return_object() noexcept -> initiation
        {
            return initiation{std::coroutine_handle<promise_type>::from_promise(*this)};
        }

        constexpr void return_value(int a) noexcept {
            returned_value = a;
        }

        // NOLINTNEXTLINE(misc-non-private-member-variables-in-classes)
        std::coroutine_handle<> previous = std::noop_coroutine();
        // NOLINTNEXTLINE(misc-non-private-member-variables-in-classes)
        int returned_value;
    };

    constexpr initiation(std::coroutine_handle<promise_type> handle) : coro(handle) {}

    // no copying
    constexpr initiation(const initiation&) = delete;

    constexpr initiation(initiation&&) = default;
    constexpr ~initiation() noexcept
    {
        coro.destroy();
    }
     
    constexpr auto await_suspend(std::coroutine_handle<> awaiting_coro) const noexcept
        -> std::coroutine_handle<>
    {
        coro.promise().previous = awaiting_coro;
        return coro;
    }

    constexpr auto await_resume() const noexcept -> int
    {
        if constexpr(not std::is_void_v<int>) {
            return coro.promise().returned_value;
        }
    }

    // NOLINTNEXTLINE(misc-non-private-member-variables-in-classes)
    std::coroutine_handle<promise_type> coro;
};



inline auto handle(int a) -> initiation
{
    co_return a;
}

inline auto handle2(int a) -> initiation
{
    co_return co_await handle(a);
}

inline auto handle3(int a) -> initiation
{
    co_return co_await handle2(a);
}

auto main() -> int
{
    auto awaitable = handle3(5);
    while(not awaitable.coro.done()) {
        awaitable.coro.resume();
    }
    auto ret = awaitable.coro.promise().returned_value;
    std::cout << ret << std::endl;
}

link to godbold

@github-actions github-actions bot added the clang Clang issues not falling into any other category label Jun 3, 2024
@cor3ntin
Copy link
Contributor

cor3ntin commented Jun 3, 2024

@ChuanqiXu9

@cor3ntin cor3ntin added the coroutines C++20 coroutines label Jun 3, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 3, 2024

@llvm/issue-subscribers-coroutines

Author: None (DarkWingMcQuack)

While clang 17.0.1 is able to elide the heap allocations of the coroutines, clang 18.1.0 does not seem to be able to do so even for simple coroutine code like this:
struct initiation : public std::suspend_always
{
    struct promise_type
    {
        constexpr auto initial_suspend() const noexcept { return std::suspend_always{}; }

        constexpr auto final_suspend() const noexcept {
            struct final_awaiter : public std::suspend_always
            {
                auto await_suspend(std::coroutine_handle&lt;initiation::promise_type&gt; h) noexcept -&gt; std::coroutine_handle&lt;&gt;
                {
                    return h.promise().previous;
                }
            };
            return final_awaiter{};
        }

        [[noreturn]] void unhandled_exception() const { std::terminate(); }

        constexpr auto get_return_object() noexcept -&gt; initiation
        {
            return initiation{std::coroutine_handle&lt;promise_type&gt;::from_promise(*this)};
        }

        constexpr void return_value(int a) noexcept {
            returned_value = a;
        }

        // NOLINTNEXTLINE(misc-non-private-member-variables-in-classes)
        std::coroutine_handle&lt;&gt; previous = std::noop_coroutine();
        // NOLINTNEXTLINE(misc-non-private-member-variables-in-classes)
        int returned_value;
    };

    constexpr initiation(std::coroutine_handle&lt;promise_type&gt; handle) : coro(handle) {}

    // no copying
    constexpr initiation(const initiation&amp;) = delete;

    constexpr initiation(initiation&amp;&amp;) = default;
    constexpr ~initiation() noexcept
    {
        coro.destroy();
    }
     
    constexpr auto await_suspend(std::coroutine_handle&lt;&gt; awaiting_coro) const noexcept
        -&gt; std::coroutine_handle&lt;&gt;
    {
        coro.promise().previous = awaiting_coro;
        return coro;
    }

    constexpr auto await_resume() const noexcept -&gt; int
    {
        if constexpr(not std::is_void_v&lt;int&gt;) {
            return coro.promise().returned_value;
        }
    }

    // NOLINTNEXTLINE(misc-non-private-member-variables-in-classes)
    std::coroutine_handle&lt;promise_type&gt; coro;
};



inline auto handle(int a) -&gt; initiation
{
    co_return a;
}

inline auto handle2(int a) -&gt; initiation
{
    co_return co_await handle(a);
}

inline auto handle3(int a) -&gt; initiation
{
    co_return co_await handle2(a);
}

auto main() -&gt; int
{
    auto awaitable = handle3(5);
    while(not awaitable.coro.done()) {
        awaitable.coro.resume();
    }
    auto ret = awaitable.coro.promise().returned_value;
    std::cout &lt;&lt; ret &lt;&lt; std::endl;
}

link to godbold

@yuxuanchen1997 yuxuanchen1997 self-assigned this Jun 3, 2024
@yuxuanchen1997
Copy link
Member

I am planning to systematically address HALO. However, I checked the optimization remarks (-Rpass=coro-elide and -Rpass-missed=coro-elide). It appears unchanged between the versions you said.

@yuxuanchen1997
Copy link
Member

I am planning to systematically address HALO. However, I checked the optimization remarks (-Rpass=coro-elide and -Rpass-missed=coro-elide). It appears unchanged between the versions you said.

Oh sorry, nevermind. I saw your godbolt.

@yuxuanchen1997
Copy link
Member

Bisect blame to #79712

@yuxuanchen1997
Copy link
Member

Actually, a much earlier commit c467245 has impact on this too.

@ChuanqiXu9
Copy link
Member

As I said in #64586, currently, the coroutine-elision is a purely middle end optimization and not a language optimization. So we don't guarantee it will happen all the time. In other words, the compiler is free to not enable coroutine elision for any reasons.

To make this compulsory, I think we have to introduce it as a language extensions or by new language features. Otherwise, I think we will always meet such questions why my code doesn't get optimized by HOLO.

And I like to close this one and tract such issues in #64586 all together.

I am planning to systematically address HALO. However, I checked the optimization remarks (-Rpass=coro-elide and -Rpass-missed=coro-elide). It appears unchanged between the versions you said.

Looking forward to it : )

@ChuanqiXu9 ChuanqiXu9 closed this as not planned Won't fix, can't repro, duplicate, stale Jun 4, 2024
@EugeneZelenko EugeneZelenko added the question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead! label Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category coroutines C++20 coroutines question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead!
Projects
None yet
Development

No branches or pull requests

6 participants