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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
