[PATCH] D124823: [OpenMPIRBuilder] Introduce OMPRegionInfo managing the stack of OpenMP region constructs.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur created this revision.
Meinersbur added reviewers: kiranchandramohan, ftynse, peixin, jdoerfert, clementval, Leporacanthicus, kiranktp, arnamoy10, bryanpkc, Chuanfeng, AMDChirag, anchu-rajendran, SouraVX, fghanim, jdenny, MatsPetersson.
Meinersbur added a project: OpenMP.
Herald added subscribers: awarzynski, sdasgup3, wenzhicui, wrengr, Chia-hungDuan, ormris, dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagainst, shauheen, rriddle, mehdi_amini, guansong, hiraditya, yaxunl.
Herald added a project: All.
Meinersbur requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1, stephenneuendorffer, nicolasvasilache.
Herald added projects: clang, MLIR, LLVM.

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 <https://reviews.llvm.org/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.

10. 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.

11 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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124823

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/critical_codegen.cpp
  clang/test/OpenMP/critical_codegen_attr.cpp
  clang/test/OpenMP/irbuilder_nested_openmp_parallel_empty.c
  clang/test/OpenMP/irbuilder_nested_parallel_for.c
  clang/test/OpenMP/masked_codegen.cpp
  clang/test/OpenMP/master_codegen.cpp
  clang/test/OpenMP/ordered_codegen.cpp
  clang/test/OpenMP/parallel_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124823.426579.patch
Type: text/x-patch
Size: 309847 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220503/c5e69324/attachment-0001.bin>