Message ID | E801220A-ED43-43AC-98F0-3BED834460A9@sandoe.co.uk |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> 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 <patchwork@sourceware.org>; 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 <gcc-patches@gcc.gnu.org>; 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 <iain@sandoe.co.uk> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.21\)) Subject: [PATCH] coroutine use of lambda capture proxies [PR96517] Message-Id: <E801220A-ED43-43AC-98F0-3BED834460A9@sandoe.co.uk> Date: Thu, 21 Oct 2021 15:31:01 +0100 To: GCC Patches <gcc-patches@gcc.gnu.org> 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 <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Cc: Nathan Sidwell <nathan@acm.org> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
coroutine use of lambda capture proxies [PR96517]
|
|
Commit Message
Iain Sandoe
Oct. 21, 2021, 2:31 p.m. UTC
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 {
<STATEMENT_LIST>{
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 <iain@sandoe.co.uk>
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