Skip to content

[MLIR][OpenMP][Docs] NFC: Reorganize 'omp' dialect documentation #107232

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 2 commits into from
Sep 16, 2024

Conversation

skatrak
Copy link
Member

@skatrak skatrak commented Sep 4, 2024

This patch creates a handwritten main documentation page for the OpenMP dialect linking to the ODS-generated one as a sub-section.

This new page can be extended to better describe overall design decisions of the dialect rather than relying exclusively on documentation generated automatically from ODS descriptions. After some investigation, there seem to be a few main ways we could structure dialect documentation to allow the introduction of possibly extensive handwritten text.

  • Create a top-level OpenMPDialect.td file that includes the auto-generated one. This is what the acc dialect currently does, but it results in the addition of two equal TOCs. It would be possible to move the include before all handwritten sections so that the page would have a single TOC, but I believe moving general descriptions to the end of the document would hurt readability. Also keeping the section order without introducing a second TOC would mean the TOC would be inserted somewhere halfway through the page, which isn't useful.
  • Create an OpenMPDialect directory with an _index.md including the auto-generated documentation. This is a different way of reproducing the same issues described above, which is what is currently done for the linalg dialect. The multiple TOC issue there is avoided by only including automatically-generated documentation for operations (i.e. mlir-tblgen -gen-op-doc) rather than for dialects (i.e. mlir-tblgen -gen-dialect-doc). That approach would make it impossible to generate all of the documentation without adding new tablegen backends for DialectAttr, DialectType and EnumAttrInfo definitions or making the TOC optional through a command line option.
  • Create an OpenMPDialect directory with an _index.md that does not include the auto-generated documentation. Instead, link to another document in that directory that includes it. This is the approach taken here, and it circumvents all these issues without having to make any changes to tablegen backends.

This patch creates a handwritten main documentation page for the OpenMP dialect
linking to the ODS-generated one as a sub-section.

This new page can be extended to better describe overall design decisions of
the dialect rather than relying exclusively on documentation generated
automatically from ODS descriptions. After some investigation, there seem to be
a few main ways we could structure dialect documentation to allow the
introduction of possibly extensive handwritten text.
  - Create a top-level OpenMPDialect.td file that includes the auto-generated
one. This is what the `acc` dialect currently does, but it results in the
addition of two equal TOCs (one of them automatically generated). It would be
possible to move the `include` before all handwritten sections so that the page
would have a single TOC, but I believe moving general descriptions to the end
of the document would hurt readability. Also keeping the section order without
introducing a second TOC would mean the TOC would be inserted somewhere halfway
through the page, which isn't useful.
  - Create an OpenMPDialect directory with an _index.md including the
auto-generated documentation. This is a different way of reproducing the same
issues described above, which is what is currently done for the `linalg`
dialect. The multiple TOC issue there is avoided by only including
automatically-generated documentation for operations (i.e. `mlir-tblgen
-gen-op-doc`) rather than for dialects (i.e. `mlir-tblgen -gen-dialect-doc`).
That approach would make it impossible to generate all of the documentation
without adding new tablegen backends for `DialectAttr`, `DialectType` and
`EnumAttrInfo` definitions or making the TOC optional through a command line
option.
  - Create an OpenMPDialect directory with an _index.md that does not include
the auto-generated documentation. Instead, link to another document in that
directory with includes it. This is the approach taken here, and it circumvents
all these issues without having to make any changes to tablegen backends.
@llvmbot
Copy link
Member

llvmbot commented Sep 4, 2024

@llvm/pr-subscribers-mlir-openmp

@llvm/pr-subscribers-mlir

Author: Sergio Afonso (skatrak)

Changes

This patch creates a handwritten main documentation page for the OpenMP dialect linking to the ODS-generated one as a sub-section.

This new page can be extended to better describe overall design decisions of the dialect rather than relying exclusively on documentation generated automatically from ODS descriptions. After some investigation, there seem to be a few main ways we could structure dialect documentation to allow the introduction of possibly extensive handwritten text.

  • Create a top-level OpenMPDialect.td file that includes the auto-generated one. This is what the acc dialect currently does, but it results in the addition of two equal TOCs. It would be possible to move the include before all handwritten sections so that the page would have a single TOC, but I believe moving general descriptions to the end of the document would hurt readability. Also keeping the section order without introducing a second TOC would mean the TOC would be inserted somewhere halfway through the page, which isn't useful.
  • Create an OpenMPDialect directory with an _index.md including the auto-generated documentation. This is a different way of reproducing the same issues described above, which is what is currently done for the linalg dialect. The multiple TOC issue there is avoided by only including automatically-generated documentation for operations (i.e. mlir-tblgen -gen-op-doc) rather than for dialects (i.e. mlir-tblgen -gen-dialect-doc). That approach would make it impossible to generate all of the documentation without adding new tablegen backends for DialectAttr, DialectType and EnumAttrInfo definitions or making the TOC optional through a command line option.
  • Create an OpenMPDialect directory with an _index.md that does not include the auto-generated documentation. Instead, link to another document in that directory that includes it. This is the approach taken here, and it circumvents all these issues without having to make any changes to tablegen backends.

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

2 Files Affected:

  • (added) mlir/docs/Dialects/OpenMPDialect/ODS.md (+3)
  • (added) mlir/docs/Dialects/OpenMPDialect/_index.md (+14)
diff --git a/mlir/docs/Dialects/OpenMPDialect/ODS.md b/mlir/docs/Dialects/OpenMPDialect/ODS.md
new file mode 100644
index 00000000000000..51c61c86ebc6e7
--- /dev/null
+++ b/mlir/docs/Dialects/OpenMPDialect/ODS.md
@@ -0,0 +1,3 @@
+# ODS Documentation
+
+[include "Dialects/OpenMPDialect.md"]
diff --git a/mlir/docs/Dialects/OpenMPDialect/_index.md b/mlir/docs/Dialects/OpenMPDialect/_index.md
new file mode 100644
index 00000000000000..f4a468ca84da99
--- /dev/null
+++ b/mlir/docs/Dialects/OpenMPDialect/_index.md
@@ -0,0 +1,14 @@
+# 'omp' Dialect
+
+The `omp` dialect is for representing directives, clauses and other definitions
+of the OpenMP programming model. This directive-based programming model, defined
+for the C, C++ and Fortran programming languages, provides abstractions to
+simplify the development of parallel and accelerated programs.
+
+Operations in this MLIR dialect generally correspond to a single OpenMP
+directive, taking arguments that represent their supported clauses, though this
+is not always the case. For a detailed information of operations, types and
+other definitions in this dialect, refer to the automatically-generated
+[ODS Documentation](ODS.md).
+
+[TOC]

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.

Except for my little nit about referring to the OpenMP website, it LGTM.

@skatrak
Copy link
Member Author

skatrak commented Sep 6, 2024

Can anyone familiar with the structure of MLIR docs pages take a quick look and let me know if this reorganization is fine? I've been able to build the mlir-www documentation webpage incorporating this change locally and it looks as intended.

@skatrak skatrak merged commit 0f86cb3 into main Sep 16, 2024
8 checks passed
@skatrak skatrak deleted the users/skatrak/mlir-omp-docs-01-reorg branch September 16, 2024 10:22
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.

3 participants