Skip to content

[MLIR][OpenMP][Docs] NFC: Document clause-based op representation #107234

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

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

skatrak
Copy link
Member

@skatrak skatrak commented Sep 4, 2024

This patch documents the clause-based op represetation discussed in this RFC.

@llvmbot
Copy link
Member

llvmbot commented Sep 4, 2024

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-openmp

Author: Sergio Afonso (skatrak)

Changes

This patch documents the clause-based op represetation discussed in this RFC.


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

1 Files Affected:

  • (modified) mlir/docs/Dialects/OpenMPDialect/_index.md (+192)
diff --git a/mlir/docs/Dialects/OpenMPDialect/_index.md b/mlir/docs/Dialects/OpenMPDialect/_index.md
index 43e82a871db066..8abdc731675fc0 100644
--- a/mlir/docs/Dialects/OpenMPDialect/_index.md
+++ b/mlir/docs/Dialects/OpenMPDialect/_index.md
@@ -39,3 +39,195 @@ mapping information is named `MapInfoOp` / `omp.map.info`. The same rules are
 followed if multiple operations are created for different variants of the same
 directive, e.g. `atomic` becomes `Atomic{Read,Write,Update,Capture}Op` /
 `omp.atomic.{read,write,update,capture}`.
+
+## Clause-Based Operation Definition
+
+One main feature of the OpenMP specification is that, even though the set of
+clauses that could be applied to a given directive is independent from other
+directives, these clauses can generally apply to multiple directives. Since
+clauses usually define which arguments the corresponding MLIR operation takes,
+it is possible (and preferred) to define OpenMP dialect operations based on the
+list of clauses taken by the corresponding directive. This makes it simpler to
+keep their representation consistent across operations and minimizes redundancy
+in the dialect.
+
+To achieve this, the base `OpenMP_Clause` tablegen class has been created. It is
+intended to be used to create clause definitions that can be then attached to
+multiple `OpenMP_Op` definitions, resulting in the latter inheriting by default
+all properties defined by clauses attached, similarly to the trait mechanism.
+This mechanism is implemented in
+[OpenMPOpBase.td](https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/OpenMP/OpenMPOpBase.td).
+
+### Adding a Clause
+
+OpenMP clause definitions are located in
+[OpenMPClauses.td](https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td).
+For each clause, an `OpenMP_Clause` subclass and a definition based on it must
+be created. The subclass must take a `bit` template argument for each of the
+properties it may define, listed below, which is forwarded to the base class.
+The definition must be an instantiation of the base class where all these
+template arguments are set to `false`. The definition's name must be
+`OpenMP_<Name>Clause`, whereas its base class' must be
+`OpenMP_<Name>ClauseSkip`. Following this pattern makes it possible to
+optionally skip the inheritance of some properties when defining operations:
+[more info](#overriding-clause-inherited-properties).
+
+Clauses can define the following properties:
+  - `list<Traits> traits`: To be used when having a certain clause always
+implies some op trait, like the `map` clause and the `MapClauseOwningInterface`.
+  - `dag(ins) arguments`: Mandatory property holding values and attributes
+used to represent the clause. Argument names use snake_case and should contain
+the clause name to avoid name clashes between clauses. Variadic arguments
+(non-attributes) must contain the "_vars" suffix.
+  - `string assemblyFormat`: Optional formatting string to produce custom
+human-friendly printers and parsers for arguments associated with the clause.
+It will be combined with assembly formats for other clauses depending on the
+`isRequired` template argument passed to the parent `OpenMP_Clause` class, as
+explained [below](#adding-an-operation). 
+  - `string description`: Optional description text to describe the clause and
+its representation.
+  - `string extraClassDeclaration`: Optional C++ declarations to be added to
+operation classes including the clause.
+
+For example:
+
+```tablegen
+class OpenMP_ExampleClauseSkip<
+    bit traits = false, bit arguments = false, bit assemblyFormat = false,
+    bit description = false, bit extraClassDeclaration = false
+  > : OpenMP_Clause</*isRequired=*/false, traits, arguments, assemblyFormat,
+                    description, extraClassDeclaration> {
+  let arguments = (ins
+    Optional<AnyType>:$example_var
+  );
+
+  let assemblyFormat = [{
+    `example` `(` $example_var `:` type($example_var) `)`
+  }];
+
+  let description = [{
+    The `example_var` argument defines the variable to which the EXAMPLE clause
+    applies.
+  }];
+}
+
+def OpenMP_ExampleClause : OpenMP_ExampleClauseSkip<>;
+```
+
+### Adding an Operation
+
+Operations in the OpenMP dialect, located in
+[OpenMPOps.td](https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td),
+can be defined like any other regular operation by just specifying a `mnemonic`
+and optional list of `traits` when inheriting from `OpenMP_Op`, and then
+defining the expected `description`, `arguments`, etc. properties inside of its
+body. However, in most cases, basing the operation definition on its list of
+accepted clauses is significantly simpler because some of the properties can
+just be inherited from these clauses.
+
+In general, the way to achieve this is to specify, in addition to the `mnemonic`
+and optional list of `traits`, a list of `clauses` where all the applicable
+`OpenMP_<Name>Clause` definitions are added. Then, the only properties that
+would have to be defined in the operation's body are the `summary` and
+`description`. For the latter, only the operation itself would have to be
+defined, and the description for its clause-inherited arguments is appended
+through the inherited `clausesDescription` property.
+
+If the operation is intended to have a single region, this is better achieved by
+setting the `singleRegion=true` template argument of `OpenMP_Op` rather manually
+populating the `regions` property of the operation, because that way the default
+`assemblyFormat` is also updated correspondingly.
+
+For example:
+
+```tablegen
+def ExampleOp : OpenMP_Op<"example", traits = [
+    AttrSizedOperandSegments, ...
+  ], clauses = [
+    OpenMP_AlignedClause, OpenMP_IfClause, OpenMP_LinearClause, ...
+  ], singleRegion = true> {
+  let summary = "example construct";
+  let description = [{
+    The example construct represents...
+  }] # clausesDescription;
+}
+```
+
+This is possible because the `arguments`, `assemblyFormat` and
+`extraClassDeclaration` properties of the operation are by default
+populated by concatenating the corresponding properties of the clauses on the
+list. In the case of the `assemblyFormat`, this also involves splitting the
+format strings for required clauses from the ones for optional clauses. The
+latter are wrapped in an `oilist()` and interleaved with "|" instead of spaces.
+
+### Overriding Clause-Inherited Properties
+
+Although the clause-based definition of operations can greatly reduce work, it's
+also somewhat restrictive, since there may be some situations where only part of
+the operation definition can be automated in that manner. For a fine-grained
+control over properties inherited from each clause two features are available:
+
+  - Inhibition of properties. By using `OpenMP_<Name>ClauseSkip` tablegen
+classes, the list of properties copied from the clause to the operation can be
+selected. For example, `OpenMP_IfClauseSkip<assemblyFormat = true>` would result
+in every property defined for the `OpenMP_IfClause` except for the
+`assemblyFormat` being used to initially populate the properties of the
+operation.
+  - Augmentation of properties. There are times when there is a need to add to
+a clause-populated operation property. Instead of overriding the property in the
+definition of the operation and having to manually replicate what would
+otherwise be automatically populated before adding to it, some internal
+properties are defined to hold this default value: `clausesArgs`,
+`clausesAssemblyFormat` and `clausesExtraClassDeclaration`.
+
+In the following example, assuming both the `OpenMP_InReductionClause` and the
+`OpenMP_ReductionClause` define a `getReductionVars` extra class declaration,
+we skip the conflicting `extraClassDeclaration`s inherited by both clauses and
+provide another implementation, without having to also re-define other
+declarations inherited from the `OpenMP_AllocateClause`:
+
+```tablegen
+def ExampleOp : OpenMP_Op<"example", traits = [
+    AttrSizedOperandSegments, ...
+  ], clauses = [
+    OpenMP_AllocateClause,
+    OpenMP_InReductionClauseSkip<extraClassDeclaration = true>,
+    OpenMP_ReductionClauseSkip<extraClassDeclaration = true>
+  ], singleRegion = true> {
+  let summary = "example construct";
+  let description = [{
+    This operation represents...
+  }] # clausesDescription;
+
+  // Override the clause-populated extraClassDeclaration and add the default
+  // back via appending clausesExtraClassDeclaration to it. This has the effect
+  // of adding one declaration. Since this property is skipped for the
+  // InReduction and Reduction clauses, clausesExtraClassDeclaration won't
+  // incorporate the definition of this property for these clauses.
+  let extraClassDeclaration = [{
+    SmallVector<Value> getReductionVars() {
+      // Concatenate inReductionVars and reductionVars and return the result...
+    }
+  }] # clausesExtraClassDeclaration;
+}
+```
+
+These features are intended for complex edge cases, but an effort should be made
+to avoid having to use them, since they may introduce inconsistencies and
+complexity to the dialect.
+
+### Tablegen Verification Pass
+
+As a result of the implicit way in which fundamental properties of MLIR
+operations are populated following this approach, and the ability to override
+them, forgetting to append clause-inherited values might result in hard to debug
+tablegen errors.
+
+For this reason, the `-verify-openmp-ops` tablegen pseudo-backend was created.
+It runs before any other tablegen backends are triggered for the
+[OpenMPOps.td](https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td)
+file and warns any time a property defined for a clause is not found in the
+corresponding operation, except if it is explicitly skipped as described
+[above](#overriding-clause-inherited-properties). This way, in case of a later
+tablegen failure while processing OpenMP dialect operations, earlier messages
+triggered by that pass can point to a likely solution.

Copy link
Contributor

@mjklemm mjklemm left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

LGTM

@skatrak skatrak force-pushed the users/skatrak/mlir-omp-docs-02-naming branch from 9affb19 to 83b1f04 Compare September 16, 2024 10:25
Base automatically changed from users/skatrak/mlir-omp-docs-02-naming to main September 16, 2024 10:25
@skatrak skatrak force-pushed the users/skatrak/mlir-omp-docs-03-clauses branch from 3e8a41f to 3197658 Compare September 16, 2024 10:27
@skatrak skatrak merged commit 551134c into main Sep 16, 2024
5 of 7 checks passed
@skatrak skatrak deleted the users/skatrak/mlir-omp-docs-03-clauses branch September 16, 2024 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants