Skip to content

[MLIR][OpenMP][Docs] Document operand structures (NFC) #108824

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 17, 2024

Conversation

skatrak
Copy link
Member

@skatrak skatrak commented Sep 16, 2024

This patch updates the OpenMP dialect top-level documentation to describe the operand structures, when they can be used and how they are automatically generated.

This patch updates the OpenMP dialect top-level documentation to describe the
operand structures, when they can be used and how they are automatically
generated.
@llvmbot
Copy link
Member

llvmbot commented Sep 16, 2024

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-openmp

Author: Sergio Afonso (skatrak)

Changes

This patch updates the OpenMP dialect top-level documentation to describe the operand structures, when they can be used and how they are automatically generated.


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

1 Files Affected:

  • (modified) mlir/docs/Dialects/OpenMPDialect/_index.md (+49)
diff --git a/mlir/docs/Dialects/OpenMPDialect/_index.md b/mlir/docs/Dialects/OpenMPDialect/_index.md
index 7a3837a661c592..88437b8cf828cc 100644
--- a/mlir/docs/Dialects/OpenMPDialect/_index.md
+++ b/mlir/docs/Dialects/OpenMPDialect/_index.md
@@ -238,6 +238,55 @@ corresponding operation, except if it is explicitly skipped as described
 tablegen failure while processing OpenMP dialect operations, earlier messages
 triggered by that pass can point to a likely solution.
 
+### Operand Structures
+
+One consequence of basing the representation of operations on the set of values
+and attributes defined for each clause applicable to the corresponding OpenMP
+directive is that operation argument lists tend to be long. This has the effect
+of making C++ operation builders difficult to work with and easy to mistakenly
+pass arguments in the wrong order, which may sometimes introduce hard to detect
+problems.
+
+A solution provided to this issue are operand structures. The main idea behind
+them is that there is one defined for each clause, holding a set of fields that
+contain the data needed to initialize each of the arguments associated with that
+clause. Clause operand structures are aggregated into operation operand
+structures via class inheritance. Then, a custom builder is defined for each
+operation taking the corresponding operand structure as a parameter. Since each
+argument is a named member of the structure, it becomes much simpler to set up
+the desired arguments to create a new operation.
+
+Ad-hoc operand structures available for use within the ODS definition of custom
+operation builders might be defined in
+[OpenMPClauseOperands.h](https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/OpenMP/OpenMPClauseOperands.h).
+However, this is generally not needed for clause-based operation definitions.
+The `-gen-openmp-clause-ops` tablegen backend, triggered when building the 'omp'
+dialect, will automatically produce structures in the following way:
+
+- It will create a `<Name>ClauseOps` structure for each `OpenMP_Clause`
+definition with one field per argument.
+- The name of each field will match the tablegen name of the corresponding
+argument, except for replacing snake case with camel case.
+- The type of the field will be obtained from the corresponding tablegen
+argument's type:
+  - Values are represented with `mlir::Value`, except for `Variadic`, which
+  makes it an `llvm::SmallVector<mlir::Value>`.
+  - `OptionalAttr` is represented by the translation of its `baseAttr`.
+  - `TypedArrayAttrBase`-based attribute types are represented by wrapping the
+  translation of their `elementAttr` in an `llvm::SmallVector`. The only
+  exception for this case is if the `elementAttr` is a "scalar" (i.e. non
+  array-like) attribute type, in which case the more generic `mlir::Attribute`
+  will be used in place of its `storageType`.
+  - For `ElementsAttrBase`-based attribute types a best effort is attempted to
+  obtain an element type (`llvm::APInt`, `llvm::APFloat` or
+  `DenseArrayAttrBase`'s `returnType`) to be wrapped in an `llvm::SmallVector`.
+  If it cannot be obtained, which will happen with non-builtin direct subclasses
+  of `ElementsAttrBase`, a warning will be emitted and the `storageType` (i.e.
+  specific `mlir::Attribute` subclass) will be used instead.
+  - Other attribute types will be represented with their `storageType`.
+- It will create `<Name>Operands` structure for each operation, which is an
+empty structure subclassing all operand structures defined for the corresponding `OpenMP_Op`'s clauses.
+
 ## Loop-Associated Directives
 
 Loop-associated OpenMP constructs are represented in the dialect as loop wrapper

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, thanks!

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

@skatrak skatrak merged commit 64cfce9 into llvm:main Sep 17, 2024
11 checks passed
@skatrak skatrak deleted the docs-op-structures branch September 17, 2024 09:37
tmsri pushed a commit to tmsri/llvm-project that referenced this pull request Sep 19, 2024
This patch updates the OpenMP dialect top-level documentation to
describe the operand structures, when they can be used and how they are
automatically generated.
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