This is an archive of the discontinued LLVM Phabricator instance.

[OpenMPIRBuilder] Introduce OMPRegionInfo managing the stack of OpenMP region constructs.
Needs ReviewPublic

Authored by Meinersbur on May 2 2022, 11:12 PM.

Details

Summary

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)
  1. Make region management private to OpenMPIRBuilder. Frontends do not need to be concerned about it.
  1. 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.
  1. 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.
  1. 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.
  1. 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.
  1. 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.
  1. 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.
  1. 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.
  1. 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.

Diff Detail