From b2665c7cb9b0a610e7620ad53c1ae1acd426bc2d Mon Sep 17 00:00:00 2001 From: Khalil Estell Date: Thu, 3 Sep 2026 11:58:35 -0700 Subject: [PATCH] :zap: (patch) De-coroutine pure-forwarder functions in the enumerator Several endpoint_io/enumerator functions never actually suspend, or only tail-forward another future with nothing local borrowed across the await - each one was still paying for a full coroutine ramp/resume/destroy triple (and, where the failure path is real, an EH unwind table entry) for no benefit over a plain function: - size_eio::driver_read/driver_write - pure synchronous bookkeeping (a fixed 0, or a length computation), never suspends at all. Converted to plain functions returning an already-complete future. - enumerator_eio::driver_read - a pure tail call to m_ctrl_ep->read() with a matching return type and nothing borrowed across the await. Converted to `return m_ctrl_ep->read(...);`. - send_error_to_host - was a coroutine only because m_retry_counter += 1 ran after the await. Reordered ahead of it (verified against every use of m_retry_counter in run()'s loop: a throwing stall unwinds run() entirely either way, so the increment's exact position relative to a *throwing* stall is unobservable), then converted to a tail call. enumerator_eio::driver_write is intentionally left as a real coroutine - its return value (p_buffer.length()) does not match what write() itself returns (see the existing TODO(#99) about actually limiting/reporting bytes written), so it isn't a pure forwarder. Also left a TODO(#3) on write_string_view, pointing at a filed issue: its scratch locals are borrowed across its own await, which is what blocks it from collapsing the same way - filed as a separate, smaller follow-up rather than folded in here. Measured end-to-end on the usb demo (stm32f103zg, clang 20, MinSizeRel): 64,757 -> 63,309 bytes of flash (-1,448 B). test_enumerator passes. --- v1/modules/enumerator.cppm | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/v1/modules/enumerator.cppm b/v1/modules/enumerator.cppm index 0a5b59b..55bd640 100644 --- a/v1/modules/enumerator.cppm +++ b/v1/modules/enumerator.cppm @@ -405,7 +405,7 @@ private: async::future driver_read(async::context& p_context, mem::scatter_span p_buffer) override { - co_return co_await m_ctrl_ep->read(p_context, p_buffer); + return m_ctrl_ep->read(p_context, p_buffer); } async::future driver_write( @@ -426,7 +426,7 @@ private: async::future driver_read(async::context&, mem::scatter_span) override { - co_return 0; + return async::future(0); } async::future driver_write( @@ -435,7 +435,7 @@ private: { auto const length = p_buffer.length(); total_length += length; - co_return length; + return async::future(length); } usize total_length = 0; @@ -501,10 +501,14 @@ private: co_await m_ctrl_ep->write(p_context, {}); } + // The increment is reordered ahead of the stall so this can tail-forward + // its future directly. Behaviorally equivalent even if stall() throws - + // run() only ever exits by propagating an exception, so nothing ever + // reads m_retry_counter again once that happens either way. hal::task send_error_to_host(async::context& p_context) { - co_await m_ctrl_ep->stall(p_context, true); m_retry_counter += 1; + return m_ctrl_ep->stall(p_context, true); } hal::task handle_standard_device_request(async::context& p_context, @@ -842,6 +846,12 @@ private: co_await send_error_to_host(p_context); } + // TODO(#3): `header` below is a stack-local borrowed by `payload` across + // the co_await, which is what keeps this a coroutine (a whole ramp/resume + // /destroy triple for one tail call). Hoisting it to a member would let + // this collapse to `return write_and_flush(...);` - see issue for the + // one caveat to confirm first (the three call sites in + // handle_str_descriptors must not have overlapping in-flight uses of it). hal::task write_string_view(async::context& p_context, std::u16string_view p_str, u16 p_max_length)