Skip to content

[OpenMPIRBuilder] Introduce OMPRegionInfo managing the stack of OpenMP region constructs. #130135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 52 commits into
base: users/meinersbur/irbuilder-extract
Choose a base branch
from

Conversation

Meinersbur
Copy link
Member

@Meinersbur Meinersbur commented Mar 6, 2025

The current OpenMPIRBuilder currently fails misrable with anything that causes control flow to jump out of an outlined region:

  • Cancellation points
  • C++ Destructors
  • C++ Exceptions

This patch introduces a stack of OMPRegionInfo objects that keeps track of which OpenMP (outlined or non-outlined) region we are currently in, and handles control flow escaping this region.

RegionStack replaces FinalizationStack with a more idiomatic modeling of every region instead of only those that have a finalization callback. It can be though as a continuation of D118409, and fixes the following problems:

  1. Avoid multiple stacks with different meaning and not necessarily overlapping regions/scopes that CGStmtOpenMP has (eg. OMPLexicalScope, OMPLoopScope, OMPPrivateScope, OMPLocalDeclMapRAII. OMPTransformDirectiveScopeRAII, OMPLoopNestStack. BreakContinueStack, OMPCancelStack, InlinedOpenMPRegionRAII. CGCapturedStmtRAII, etc)
  2. Make region management private to OpenMPIRBuilder. Frontends do not need to be concerned about it.
  3. FinalizeCallbackTy now never inserts branches to rejoin control flow, which is the job of the region-handling method. Previously, this was the job of the "last" finalization callback, making them hardly composable. Some problem with it sometimes called for cancellation control flow only, sometimes for regular exits as well.
  4. Make call of the finalization callback more predictable. It is always called inside the createXYZ function and after the BodeGenCallabck. This also allows passing it as llvm::function_ref instead of std::function.
  5. Reduce InsertPoint/Instruction/BasicBlock invalidation problems. For instance, adding and later removing an unreachable instruction causes problem when another part stored a reference to it, for instance inside a InsertPoint. Similar issues with MergeBlockIntoPredecessor. A FinalizeCallbackTy call might also insert new control flow such that any InsertPoint after it become invalidated. Instead, we create a new block (e.g. by splitting) behind the callback Insert point before calling which remains stable irrespectively to what the callback does.
  6. Reduce the need for the IR/InsertPoint to be any particular form: with terminator/degenerate, InsertPoint at the beginning of a block/before the terminator/at the end, etc. Different behavior depending on where the InsertPoint makes the code fragile to changes.
  7. Groundwork for future support of cancellation (or other irregular region exists) in loops. Within the BodyGen callback of createCanonicalLoop it is not yet known which directive it is associated with, which is only know when the applyXYZ method is called. Hance, handling of irregular exits that depend on the kind of region must be delayed.
  8. The entire region nest can be introspected via the RegionStack containing all regions, not just the ones with a registered finalization callback. Potential support for leaving multiple regions at once, as #pragma omp cancel parallel within a #pragma omp for would.
  9. Don't use OpenMPIRBuilder at all if inside a region not handled by OpenMPIRBuilder. The PushAndPopStackRAII (what is the meaning of that name?) solution was incomplete and required access to private members. NonOpenMPIRBuilderRegion handles this in Clang itself. However, the more complete solution would be to not use OpenMPIRBuilder if there are any unsupported constructs in a function, as non-OpenMPIRBuilder regions within OpenMPIRBuilder-handled pose problems as well.
  10. Fix bug causing a deadlock after cancellation in a parallel region due to disagreement of how often the barrier function is called. Handling for this is moved inside the createParallel instead by each potentially cancelling directive.

PR Stack:

Originally submitted as https://reviews.llvm.org/D124823

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant