From patchwork Thu Oct 21 14:31:01 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Iain Sandoe X-Patchwork-Id: 46500 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2B39E3857C65 for ; Thu, 21 Oct 2021 14:31:24 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp001-out.apm-internet.net (smtp001-out.apm-internet.net [85.119.248.222]) by sourceware.org (Postfix) with ESMTPS id 6890E3858C60 for ; Thu, 21 Oct 2021 14:31:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6890E3858C60 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sandoe.co.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=sandoe.co.uk Received: (qmail 93300 invoked from network); 21 Oct 2021 14:31:01 -0000 X-APM-Out-ID: 16348266619329 X-APM-Authkey: 257869/1(257869/1) 5 Received: from unknown (HELO ?192.168.1.214?) (81.138.1.83) by smtp001.apm-internet.net with SMTP; 21 Oct 2021 14:31:01 -0000 From: Iain Sandoe Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.21\)) Subject: [PATCH] coroutine use of lambda capture proxies [PR96517] Message-Id: Date: Thu, 21 Oct 2021 15:31:01 +0100 To: GCC Patches X-Mailer: Apple Mail (2.3445.104.21) X-Spam-Status: No, score=-15.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_COUK, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Nathan Sidwell Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Hi (apologies to Jason and Nathan for two copies - I sent from the wrong email and omitted patches@) This is a description of how the revised handling of coroutine proxy variables is supposed to work, in the case of methods and lambdas. Given that this came up again recently, perhaps I might put some version of this description as a block comment before the rewrite code. There is a bug-fix patch for PR96517, the long preamble is to put context (and, hopefully, thus identify any more assumptions that I might have missed). ==== NOTE: It is the user’s responsibility to ensure that ‘this’ and any lambda object persist for the duration of the coroutine. IMO this is probably not the best design decision ever made by the committee since it is very easy to make innocent-looking but broken coroutine code with lambdas. [aside: I think a more user-friendly decision would have been to extend the lifetime of temporaries in statements until *all* the code before the semi-colon is executed (i.e. including deferred code in a coroutine)] anyway… consider this illustrative-only case (where ‘coroutine’ is a suitable implementation of the customisation points). I have elided some of the implementation variables, they are handled the same way as the ones shown and this is already long enough. ==== code struct test { int a; void foo() { int c; auto lam = [this, c](int b) -> coroutine { co_return a + b + c; }; } }; Before the coroutine transformation process: closure object: { int __c test* __this typedef closure_type } inline coroutine test::foo::…..::operator() (closure_type *__closure, int b) { BIND_EXPR VARS { const int c; // lambda proxy var => __closure->__c test* const this; // lambda proxy var => __clousre->__this } BODY { { const int c; test* const this; co_return this->a + b + c; } } } Now because of the design decision mentioned above, we do not copy the closure contents, only the pointer. === after transformation our coroutine frame will look like this… test::foo::……..Frame { void (*)test::foo::....Frame*)_Coro_resume_fn)(test::foo::…..Frame*) void (*)test::foo::...Frame*)_Coro_destroy_fn)(est::foo::…...Frame*) coroutine::promise_type _Coro_promise ... closure_type* const __closure // parm copy int b // parm copy, no need for mangling. ... std::__n4861::suspend_always Is_1_1 // local var frame entries are mangled with their scope info, std::__n4861::suspend_never Fs_1_3 } our rewritten lambda looks like this: BIND_EXPR VARS { void (*_Coro_resume_fn)(test::foo::._anon_4::.....Frame*); // coroutine proxy var for resume fn void (*_Coro_destroy_fn)(test::foo::._anon_4::.....Frame*); // ... coroutine::promise_type _Coro_promise; // for promise DVE => Frame->_Coro_promise ... closure_type* const __closure; // coro proxy var for closure parm DVE => Frame->__closure int b; // coro proxy var for b parm …. } BODY { { void (*_Coro_resume_fn)(test::foo::._anon_4::.....Frame*); void (*_Coro_destroy_fn)(test::foo::._anon_4::.....Frame*); coroutine::promise_type _Coro_promise; ... const test::foo::._anon_4* const __closure; int b; ... try { co_await initial suspend expr. BIND_EXPR (original lambda body) VARS { const int c; // lambda proxy var; DVE => __closure->__c test* const this; // lambda proxy var DVE => __closure->__this } BODY { { const int c; test* const this; co_return this->a + b + c; } } } catch(...) cleanup_stmt … } c-finally ... final.suspend:; … } } We have a predicate for is_capture_proxy() for lambdas So .. in this expression: co_return this->a + b + c; If we ask “is_capture_proxy” for ’this’ or ‘c’ we should answer “yes” - these vars are referred to via the closure object and, if the constraints of the standard are met (lifetime of the closure object and the captured this), then it should be just as if they were used in the body of a regular lambda. The closure pointer itself is a coro proxy var (for the parm) b is a coro proxy var (for the parm) Note that the parm proxies are VAR decls not PARMs (we need to make a copy, because the original parms are used in the ramp, and obviously they are *not* parms of the helper function). so when all the DECL_VALUE_EXPRs are expanded we would get: Frame->__closure->__this->a + Frame->b + Frame->_closure->__c; Whilst the initial intention of this was to make debug work (since GCC’s debug generation already knows about DVEs) it turns out that it greatly simplifies the code, since we are dealing with the original variables instead of replacing them with a component ref. PR 96517 fires because we answer “NO” for is_capture_proxy in the rewritten code, because we do not (correctly) answer “yes” for LAMBDA_TYPE_P for the rewritten function. What we need to ask is “was the original function a lambda type?”. This is what the patch does. tested on x86_64-darwin, OK for master and affected backports? thanks Iain ===== commit log When we query is_capture_proxy(), and the scope of the var is one of the two coroutine helpers, we need to look for the scope information that pertains to the original function (represented by the ramp now). We can look up the ramp function from either helper (in practice, the only caller would be the actor) and if that lookup returns NULL, it means that the coroutine component is the ramp already and handled by the usual code path. Signed-off-by: Iain Sandoe gcc/cp/ChangeLog: PR c++/96517 * lambda.c (is_capture_proxy): When the scope of the var to be tested is a coroutine helper, lookup the scope information from the parent (ramp) function. --- gcc/cp/lambda.c | 6 +++++- -- diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c index 2e9d38bbe83..c1556480e22 100644 --- a/gcc/cp/lambda.c +++ b/gcc/cp/lambda.c @@ -244,7 +244,11 @@ is_capture_proxy (tree decl) && !(DECL_ARTIFICIAL (decl) && DECL_LANG_SPECIFIC (decl) && DECL_OMP_PRIVATIZED_MEMBER (decl)) - && LAMBDA_FUNCTION_P (DECL_CONTEXT (decl))); + && (LAMBDA_FUNCTION_P (DECL_CONTEXT (decl)) + || (DECL_DECLARES_FUNCTION_P (DECL_CONTEXT (decl)) + && DECL_COROUTINE_P (DECL_CONTEXT (decl)) + && DECL_RAMP_FN (DECL_CONTEXT (decl)) + && LAMBDA_FUNCTION_P (DECL_RAMP_FN (DECL_CONTEXT (decl)))))); } /* Returns true iff DECL is a capture proxy for a normal capture