This is an archive of the discontinued LLVM Phabricator instance.

[coroutines] Implement correct GRO lifetime
ClosedPublic

Authored by GorNishanov on Apr 4 2017, 9:18 AM.
Tokens
"Mountain of Wealth" token, awarded by GorNishanov.

Details

Summary

Sema creates a declaration for gro variable as:

auto $gro = $promise.get_return_object();

However, gro variable has to outlive coroutine frame and coroutine promise, but,
it can only be initialized after the coroutine promise was created, thus, we
split its emission in two parts: EmitGroAlloca emits an alloca and sets up
the cleanups. Later when the coroutine promise is available we initialize
the gro and set the flag that the cleanup is now active.

Diff Detail

Event Timeline

GorNishanov created this revision.Apr 4 2017, 9:18 AM
rsmith added a subscriber: rsmith.May 22 2017, 11:45 AM
rsmith added inline comments.
lib/CodeGen/CGCoroutine.cpp
366

It doesn't seem safe to assume that a prior EHCleanupScope would be for the GRO variable, nor that it can only have one such cleanup. How about this: grab a stable EHStack iterator before you emit cleanups, grab another afterwards, and iterate over the cleanups in that range modifying them as appropriate.

Even that seems a bit fragile, though. Would it be feasible to thread the 'active' flag through EmitAutoVarCleanups (perhaps add it to AutoVarEmission)?

Remember old_top before emitting the cleanup and walk set Active on all emitted cleanups.
I tried to add Active flag to Emission, but it ended up being very hairy, so I went with the first option suggested.

GorNishanov marked an inline comment as done.May 22 2017, 6:02 PM

@rsmith, better now? :)

merged with top of the trunk

merge to top of the trunk

merge with tot, preparing to land

This revision is now accepted and ready to land.May 23 2017, 7:42 PM
GorNishanov closed this revision.May 23 2017, 7:43 PM

Closed by commit rL303716: [coroutines] Implement correct GRO lifetime (authored by GorNishanov)