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

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D70258#1788825 <https://reviews.llvm.org/D70258#1788825>, @rjmccall wrote:

> So it's never that OpenMP has things that need to be finalized before exiting (e.g. if something in frontend-emitted code throws an exception), it's just that OpenMP might need to generate its own control flow that breaks through arbitrary scopes in the frontend?


Depending on how you look at it. OpenMP has to finalize some constructs, which usually means to place a call before we continue with the code that follows. It also introduce a new scope in which things like privatization happen, however these are almost completely handled through clang (see below). The one thing that comes to mind is `lastprivate` which is handled by OpenMP.

Btw. Exceptions have to be caught inside an OpenMP scope that has a finalizer as it would be UB otherwise (as with the `return`).

> I would really hope that the OpenMP implementation in Clang would've used Clang's cleanup stack rather than inventing its own mechanism.

This patch uses (parts of) clangs cleanup logic and the existing CGOpenMP cleanup code (which is the existing OpenMP specific stack (`CodeGenFunction::OMPCancelStack`) this one will replace).

---

In the code that glues the old CGOpenMP to the new OMPBuilder, where we create the finalization callback, the existing `getOMPCancelDestination` is used as shown below.

  CodeGenFunction::JumpDest Dest = CGF.getOMPCancelDestination(OMPD_parallel);
  CGF.EmitBranchThroughCleanup(Dest);

In the follow up patch (D70290 <https://reviews.llvm.org/D70290>) that will lower `#pragma omp parallel` through the OMPIRBuilder, we don't need the `clang::CodeGenFunction::OMPCancelDestination`/`clang::CodeGenFunction::OMPCancelStack` stuff anymore (for the parallel) and the finalization callback becomes:

  CodeGenFunction::JumpDest Dest = getJumpDestInCurrentScope(DestBB);
  EmitBranchThroughCleanup(Dest);

With `DestBB` defined as the end of the OpenMP region/scope.

---

The additional stack is also needed because depending on the nesting we may or may not create control flow that can jump to the end of a scope. So

  #pragma omp parallel
  {
    {
      ... ;
      #pragma omp barrier // or an implicit barrier
      ... ;
    }
    parallel_exit:
  }

will cause a control flow edge from the barrier to `parallel_exit`. This edge may not exist if there is another OpenMP region nested in the parallel.

---

In D70258#1788854 <https://reviews.llvm.org/D70258#1788854>, @ABataev wrote:

> In D70258#1788825 <https://reviews.llvm.org/D70258#1788825>, @rjmccall wrote:
>
> > I would really hope that the OpenMP implementation in Clang would've used Clang's cleanup stack rather than inventing its own mechanism.
>
>
> John, current implementation completely relies on Clang's cleanup stack.


Let me rephrase the above statement less misleading:
The current implementation relies on Clang's cleanup stack *and* the OpenMP specific `clang::CodeGenFunction::OMPCancelStack`, which is what this patch will eventually replace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258