[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert marked 4 inline comments as done.
jdoerfert added a comment.

ping



================
Comment at: llvm/include/llvm/Frontend/OpenMPIRBuilder.h:69-71
+  void pushFinalizationCB(FinalizationInfo &&FI) {
+    FinalizationStack.emplace_back(std::move(FI));
+  }
----------------
jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > jdoerfert wrote:
> > > > > ABataev wrote:
> > > > > > Maybe `push...` for const reference and `emplace...` for variadic template (just like in standard library)?
> > > > > 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.
> > > > You can use it in `emplace` version while `push...` function will rely on copy semantics.
> > > I only call this once, why do I need multiple versions?
> > I thought that we may need it later in other cases, no?
> 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.
I'll make this a const reference and push back.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70258/new/

https://reviews.llvm.org/D70258