Page MenuHomePhabricator

[OpenMP][IR-Builder] Introduce the finalization stack
Needs ReviewPublic

Authored by jdoerfert on Thu, Nov 14, 11:01 AM.



As a permanent and generic solution to the problem of variable
finalization (destructors, lastprivate, ...), this patch introduces the
finalization stack. The objects on the stack describe (1) the
(structured) regions the OpenMP-IR-Builder is currently constructing,
(2) if these are cancellable, and (3) the callback that will perform the
finalization (=cleanup) when necessary.

As the finalization can be necessary multiple times, at different source
locations, the callback takes the position at which code is currently
generated. This position will also encode the destination of the "region
exit" block *iff* the finalization call was issues for a region
generated by the OpenMPIRBuilder. For regions generated through the old
Clang OpenMP code geneneration, the "region exit" is determined by Clang
inside the finalization call instead (see getOMPCancelDestination).

As a first user, the parallel + cancel barrier interaction is changed.
In contrast to the temporary solution before, the barrier generation in
Clang does not need to be aware of the "CancelDestination" block.
Instead, the finalization callback is and, as described above, later
even that one does not need to be.

D70109 will be updated to use this scheme.

Event Timeline

jdoerfert created this revision.Thu, Nov 14, 11:01 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptThu, Nov 14, 11:01 AM
ABataev added inline comments.Thu, Nov 14, 11:24 AM

Better to not capture all the object implicitly by reference, provide explicit list


Use InsertPointGuard where possible rather than use saveIP/restoreIP explicitly


Add a message for the assert.


Again, need RAII


Maybe push... for const reference and emplace... for variadic template (just like in standard library)?

jdoerfert marked 5 inline comments as done.

Addressed comments


There is no appropriate scope to put it in.


That would defeat the purpose of using move semantic. I can make it a push but we will then have to copy and not move.

ABataev added inline comments.Fri, Nov 15, 7:26 AM

You can check for OMPBuilder in the constructor/destructor.


You can use it in emplace version while push... function will rely on copy semantics.

jdoerfert marked an inline comment as done.Fri, Nov 15, 9:29 AM
jdoerfert added inline comments.

I only call this once, why do I need multiple versions?

jdoerfert updated this revision to Diff 229573.Fri, Nov 15, 9:36 AM

Replace conditional by RAII object

ABataev added inline comments.Fri, Nov 15, 9:41 AM

I thought that we may need it later in other cases, no?

ABataev added inline comments.Fri, Nov 15, 9:44 AM

Most of this code can be moved to the constructor of the PopStackRAII.

jdoerfert updated this revision to Diff 229582.Fri, Nov 15, 9:51 AM
jdoerfert marked an inline comment as done.

Make it a specialized push-pop RAII object (just moved code)

jdoerfert marked an inline comment as done.Fri, Nov 15, 9:52 AM
jdoerfert added inline comments.

We only need this interface for situations where we want to test finalization in the presence of cancellation points that are generated by the OpenMPIRBuilder while the cancelled region is not. For example, once D70290 is in, we can remove the call from the barrier code (it will be part of D70290). If we always port the regions first (sections, for, ...) we don't need this interface at all.

jdoerfert marked 4 inline comments as done.Thu, Nov 21, 9:05 AM



I'll make this a const reference and push back.