[OpenMP][Draft] Breaking up of combined/composite directives

[kparzysz]

Goal
Given a combined/composite construct, together with a list of clauses applied to the entire construct, construct a list of leaf constructs, and assign clauses to these leafs to which they apply.
The initial intent was to minimize the impact of such splitting on the existing lowering code that would use this information. At the moment the existing code deals with parser classes directly, so this implementation tried to preserve as much of it as possible (see the "to do" section).

Background
Many clauses can only be applied to a single leaf directive. For cases of clauses that could apply to multiple leaf constructs, the OpenMP specification provides detailed explanation of their meaning. In many cases the handling of a clause depends on the presence/absence of specific directives, and in case of clauses containing variables, on other clauses that contain the same variables[1]. In some cases a clause may have an effect as if another clause (not present on the combined/composite construct) was applied to some leaf directive.

Implementation
Create a class DirectiveInfo which represents a leaf directive together with the list of clauses that apply to it. Create a class CompositeInfo, which contains a list of DirectiveInfo objects. The list is initially empty, then it's populated with leaf directives without any clauses, then for each clause present in the combined/composite construct, the clause is examined to apply its effects. Typically the effect is appending the clause to the appropriate DirectiveInfo object.

The basic outline of what's going on is:

for (dir : leafs_of(combined/composite))
  leafs.append(DirectInfo(dir));

for (clause : clauses_on(combined/composite))
  apply(clause);  // attach it to the right leaf

There is more, as certain clauses have a lengthy list of conditions as to what their effect is exactly, but this is the underlying idea.

To simplify handling of cases like [1], create a map where an object (represented by semantics::Symbol) maps to the set of clauses that mention it. This is implemented via addSymsToMap functions.

The actual application of the clauses is done by the set of overloaded applyClause functions.

To do
Fill in the implementation gaps. One case is creating SHARED clauses that are implied in several cases. This would require duplicating parser::OmpObject objects, which may be somewhat laborious. As an alternative, we could change the representation of clauses from parser::OmpClause to something easier to manipulate, more specifically using semantics::Symbol instead of parser::OmpObject but it would require modifying of the existing code.

There are some additional to-do items:

• revisiting the leaf representation in the .td file (use class names instead of strings),
• making the rest of the code actually use this,
• writing unit tests (using debug dumps to express the exact clause assignment?),
• etc

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 6aed6cc40ec0006bb43f1ec4b2ec87702392ad6e 5ae6a2021aa8d449f752b72010c26c0aa5e7cd69 -- flang/lib/Lower/OpenMP.cpp flang/tools/bbc/bbc.cpp llvm/include/llvm/TableGen/DirectiveEmitter.h llvm/utils/TableGen/DirectiveEmitter.cpp

View the diff from clang-format here.

diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index be9531b574..a03ba69a82 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -590,8 +590,7 @@ static bool applyToOutermost(llvm::omp::Clause clauseId,
 template <typename Predicate>
 static bool applyIf(llvm::omp::Clause clauseId,
                     const Fortran::parser::OmpClause *clauseNode,
-                    CompositeInfo &compInfo,
-                    Predicate shouldApply) {
+                    CompositeInfo &compInfo, Predicate shouldApply) {
   bool applied = false;
   uint32_t version = getOpenMPVersion(compInfo.mod);
   for (DirectiveInfo &dir : compInfo.leafs) {

[skatrak]

One concern I have about the general approach is whether it makes sense to do this for both combined and composite constructs. For combined constructs I think it's clear this is useful, since each leaf construct can be lowered independently, so each of them would also get its applicable set of clauses.

However, composite constructs will have to be dealt with as a whole, rather than what we currently do (basically treat them as combined constructs during PFT to MLIR lowering). It looks like we might even be going to follow the route of representing each composite construct as its own MLIR operation, as per @kiranchandramohan's comments in #79843.

Having that in mind, would it make sense to, for the purposes of this patch, consider composite constructs as leaves themselves? That should probably reduce the number of cases where you'd have to insert new clauses or create some sort of higher-level representation, as discussed in yesterday's community call (hopefully to zero, removing the need for that). Then, during lowering of each composite construct we could look at these corner cases.

[skatrak]

Wouldn't it be possible to use the sets defined in "flang/include/flang/Semantics/openmp-directive-sets.h" for this? If others are needed, I think they could be added there as well.

[skatrak]

Nit: Since this is part of the flang subproject, I think it's fine to skip C/C++-only directives, like anything including 'for'.

[kparzysz]

Yes, I think it would be nice to have flags in the .td file whether a given construct is composite, compound, worksharing, worksharing-loop, etc.

[skatrak]

Nit: Probably would make sense to move this to "flang/include/flang/Tools/CrossToolHelpers.h"

[kparzysz]

I think it could go into more generic place, since this only requires MLIR, but I agree it can be useful to share this.