Skip to content
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

[Frontend][OpenMP] Add functions for checking construct type #87258

Merged
merged 19 commits into from
Apr 23, 2024

Conversation

kparzysz
Copy link
Contributor

@kparzysz kparzysz commented Apr 1, 2024

Implement helper functions to identify leaf, composite, and combined constructs.

skatrak and others added 10 commits March 27, 2024 12:15
This patch introduces a set of composable structures grouping the MLIR operands
associated to each OpenMP clause. This makes it easier to keep the MLIR
representation for the same clause consistent throughout all operations that
accept it.

The relevant clause operand structures are grouped into per-operation
structures using a mixin pattern and used to define new operation constructors.
These constructors can be used to avoid having to get the order of a possibly
large list of operands right.

Missing clauses are documented as TODOs, as well as operands which are part of
the relevant operation's operand structure but cannot be attached to the
associated operation yet, due to missing op arguments to its MLIR definition.

A follow-up patch will update Flang lowering to make use of these structures,
simplifying the passing of information from clause processing to operation-
generating functions and also simplifying the creation of operations through
the use of the new operation constructors.
This patch updates Flang lowering to use the new set of OpenMP clause operand
structures and their groupings into directive-specific sets of clause operands.

It simplifies the passing of information from the clause processor and the
creation of operations.

The `DataSharingProcessor` is slightly modified to not hold delayed
privatization state. Instead, optional arguments are added to `processStep1`
which are only passed when delayed privatization is used. This enables using
the clause operand structure for `private` and removes the need for the ad-hoc
`DelayedPrivatizationInfo` structure.

The processing of the `schedule` clause is updated to process the `chunk`
modifier rather than requiring two separate calls to the `ClauseProcessor`.

Lowering of a block-associated `ordered` construct is updated to emit a TODO
error if the `simd` clause is specified, since it is not currently supported by
the `ClauseProcessor` or later compilation stages.

Removed processing of `schedule` from `omp.simdloop`, as it doesn't apply to
`simd` constructs.
This patch performs several cleanups with the main purpose of normalizing the
code patterns used to trigger codegen for MLIR OpenMP operations and making the
processing of clauses and constructs independent. The following changes are
made:

- Clean up unused `directive` argument to `ClauseProcessor::processMap()`.
- Move general helper functions in OpenMP.cpp to the appropriate section of the
file.
- Create `gen<OpName>Clauses()` functions containing the clause processing code
specific for the associated OpenMP construct.
- Update `gen<OpName>Op()` functions to call the corresponding
`gen<OpName>Clauses()` function.
- Sort calls to `ClauseProcessor::process<ClauseName>()` alphabetically, to
avoid inadvertently relying on some arbitrary order. Update some tests that
broke due to the order change.
- Normalize `genOMP()` functions so they all delegate the generation of MLIR to
`gen<OpName>Op()` functions following the same pattern.
- Only process `nowait` clause on `TARGET` constructs if not compiling for the
target device.

A later patch can move the calls to `gen<OpName>Clauses()` out of
`gen<OpName>Op()` functions and passing completed clause structures instead, in
preparation to supporting composite constructs. That will make it possible to
reuse clause processing for a given leaf construct when appearing alone or in a
combined or composite construct, while controlling where the associated code is
produced.
This patch simplifies the lowering from PFT to MLIR of OpenMP compound
constructs (i.e. combined and composite).

The new approach consists of iteratively processing the outermost leaf
construct of the given combined construct until it cannot be split further.
Both leaf constructs and composite ones have `gen...()` functions that are
called when appropriate.

This approach enables treating a leaf construct the same way regardless of if
it appeared as part of a combined construct, and it also enables the lowering
of composite constructs as a single unit.

Previous corner cases are now handled in a more straightforward way and
comments pointing to the relevant spec section are added. Directive sets are
also completed with missing LOOP related constructs.
This removes the last use of genOmpObectList2, which has now been removed.
Emit a special leaf constuct table in DirectiveEmitter.cpp, which will
allow both decomposition of a construct into leafs, and composition of
constituent constructs into a single compound construct (is possible).
Implement helper functions to identify leaf, composite, and combined
constructs.
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 1, 2024

@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)

Changes

Implement helper functions to identify leaf, composite, and combined constructs.


Full diff: https://github.com/llvm/llvm-project/pull/87258.diff

3 Files Affected:

  • (modified) llvm/include/llvm/Frontend/OpenMP/OMP.h (+4)
  • (modified) llvm/lib/Frontend/OpenMP/OMP.cpp (+25)
  • (modified) llvm/unittests/Frontend/OpenMPComposeTest.cpp (+26)
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.h b/llvm/include/llvm/Frontend/OpenMP/OMP.h
index 4ed47f15dfe59e..ec8ae68f1c2ca0 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.h
@@ -20,6 +20,10 @@
 namespace llvm::omp {
 ArrayRef<Directive> getLeafConstructs(Directive D);
 Directive getCompoundConstruct(ArrayRef<Directive> Parts);
+
+bool isLeafConstruct(Directive D);
+bool isCompositeConstruct(Directive D);
+bool isCombinedConstruct(Directive D);
 } // namespace llvm::omp
 
 #endif // LLVM_FRONTEND_OPENMP_OMP_H
diff --git a/llvm/lib/Frontend/OpenMP/OMP.cpp b/llvm/lib/Frontend/OpenMP/OMP.cpp
index dd99d3d074fd1e..98d7c63bb8537e 100644
--- a/llvm/lib/Frontend/OpenMP/OMP.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMP.cpp
@@ -78,4 +78,29 @@ Directive getCompoundConstruct(ArrayRef<Directive> Parts) {
     return Found;
   return OMPD_unknown;
 }
+
+bool isLeafConstruct(Directive D) { return getLeafConstructs(D).empty(); }
+
+bool isCompositeConstruct(Directive D) {
+  // OpenMP Spec 5.2: [17.3, 8-9]
+  // If directive-name-A and directive-name-B both correspond to loop-
+  // associated constructs then directive-name is a composite construct
+  llvm::ArrayRef<Directive> Leafs{getLeafConstructs(D)};
+  if (Leafs.empty())
+    return false;
+  if (getDirectiveAssociation(Leafs.front()) != Association::Loop)
+    return false;
+
+  size_t numLoopConstructs =
+      llvm::count_if(Leafs.drop_front(), [](Directive L) {
+        return getDirectiveAssociation(L) == Association::Loop;
+      });
+  return numLoopConstructs != 0;
+}
+
+bool isCombinedConstruct(Directive D) {
+  // OpenMP Spec 5.2: [17.3, 9-10]
+  // Otherwise directive-name is a combined construct.
+  return !getLeafConstructs(D).empty() && !isCompositeConstruct(D);
+}
 } // namespace llvm::omp
diff --git a/llvm/unittests/Frontend/OpenMPComposeTest.cpp b/llvm/unittests/Frontend/OpenMPComposeTest.cpp
index 29b1be4eb3432c..cc02af8bf67c26 100644
--- a/llvm/unittests/Frontend/OpenMPComposeTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPComposeTest.cpp
@@ -39,3 +39,29 @@ TEST(Composition, GetCompoundConstruct) {
   Directive C7 = getCompoundConstruct({OMPD_parallel_for, OMPD_simd});
   ASSERT_EQ(C7, OMPD_parallel_for_simd);
 }
+
+TEST(Composition, IsLeafConstruct) {
+  ASSERT_TRUE(isLeafConstruct(OMPD_loop));
+  ASSERT_TRUE(isLeafConstruct(OMPD_teams));
+  ASSERT_FALSE(isLeafConstruct(OMPD_for_simd));
+  ASSERT_FALSE(isLeafConstruct(OMPD_distribute_simd));
+}
+
+TEST(Composition, IsCompositeConstruct) {
+  ASSERT_TRUE(isCompositeConstruct(OMPD_distribute_simd));
+  ASSERT_FALSE(isCompositeConstruct(OMPD_for));
+  ASSERT_TRUE(isCompositeConstruct(OMPD_for_simd));
+  // directive-name-A = "parallel", directive-name-B = "for simd",
+  // only directive-name-A is loop-associated, so this is not a
+  // composite construct, even though "for simd" is.
+  ASSERT_FALSE(isCompositeConstruct(OMPD_parallel_for_simd));
+}
+
+TEST(Composition, IsCombinedConstruct) {
+  // "parallel for simd" is a combined construct, see comment in
+  // IsCompositeConstruct.
+  ASSERT_TRUE(isCombinedConstruct(OMPD_parallel_for_simd));
+  ASSERT_FALSE(isCombinedConstruct(OMPD_for_simd));
+  ASSERT_TRUE(isCombinedConstruct(OMPD_parallel_for));
+  ASSERT_FALSE(isCombinedConstruct(OMPD_parallel));
+}

Copy link
Contributor

@skatrak skatrak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, based on the current proposal in #87247 this LGTM. If that changes, this will need another review.

Copy link
Contributor

@skatrak skatrak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just small comments. Thanks!

llvm/unittests/Frontend/OpenMPCompositionTest.cpp Outdated Show resolved Hide resolved
Base automatically changed from users/kparzysz/spr/a06-leafsorcomposite to main April 22, 2024 19:41
@kparzysz kparzysz merged commit 70d3ddb into main Apr 23, 2024
4 checks passed
@kparzysz kparzysz deleted the users/kparzysz/spr/a07-construct-type branch April 23, 2024 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:openmp OpenMP related changes to Clang flang:openmp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants