-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[MLIR][OpenMP] Add TableGen pseudo-generator for OpenMP-specific verification #95552
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
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| // Tablegen tests for the verification of clause-based OpenMP dialect operation | ||
| // definitions. | ||
|
|
||
| // Run tablegen to generate OmpCommon.td in temp directory first. | ||
| // RUN: mkdir -p %t/mlir/Dialect/OpenMP | ||
| // RUN: mlir-tblgen --gen-directive-decl --directives-dialect=OpenMP \ | ||
| // RUN: %llvm_src_root/include/llvm/Frontend/OpenMP/OMP.td \ | ||
| // RUN: -I %llvm_src_root/include > %t/mlir/Dialect/OpenMP/OmpCommon.td | ||
|
|
||
| // RUN: not mlir-tblgen -verify-openmp-ops -I %mlir_src_root/include -I %t %s 2>&1 | FileCheck %s | ||
|
|
||
| include "mlir/Dialect/OpenMP/OpenMPOpBase.td" | ||
|
|
||
|
|
||
| def OpenMP_SimpleClause : OpenMP_Clause< | ||
| /*isRequired=*/true, /*traits=*/false, /*arguments=*/false, | ||
| /*assemblyFormat=*/false, /*description=*/false, | ||
| /*extraClassDeclaration=*/false> { | ||
| let arguments = (ins I32:$val1); | ||
| let assemblyFormat = "`val1` `(` $val1 `)`"; | ||
| let description = "Simple clause description."; | ||
| let extraClassDeclaration = "void simpleClauseExtraClassDecl();"; | ||
| } | ||
|
|
||
|
|
||
| // ----------------------------------------------------------------------------- | ||
| // Verify errors / warnings for overriding each field. | ||
| // ----------------------------------------------------------------------------- | ||
|
|
||
| def 1OverrideArgsOp : OpenMP_Op<"op", clauses=[OpenMP_SimpleClause]> { | ||
| let description = "Description of operation." # clausesDescription; | ||
| dag arguments = (ins I32:$myval); | ||
| } | ||
| // CHECK: warning: 'Simple' clause-defined argument 'I32:$val1' not present in operation. | ||
| // CHECK-SAME: Consider `dag arguments = !con(clausesArgs, ...)` or explicitly skipping this field. | ||
| // CHECK-NEXT: def 1OverrideArgsOp | ||
|
|
||
| def 2OverrideAssemblyFormatOp : OpenMP_Op<"op", clauses=[OpenMP_SimpleClause]> { | ||
| let description = "Description of operation." # clausesDescription; | ||
| string assemblyFormat = "`alt_repr` `(` $val1 `)`"; | ||
| } | ||
| // CHECK: warning: 'Simple' clause-defined `assemblyFormat` not present in operation. | ||
| // CHECK-SAME: Consider concatenating `clausesAssemblyFormat` or explicitly skipping this field. | ||
| // CHECK-NEXT: def 2OverrideAssemblyFormatOp | ||
|
|
||
| def 3OverrideDescriptionOp : OpenMP_Op<"op", clauses=[OpenMP_SimpleClause]> { | ||
| let description = "Description of operation."; | ||
| } | ||
| // CHECK: error: 'Simple' clause-defined `description` not present in operation. | ||
| // CHECK-SAME: Consider concatenating `clausesDescription` or explicitly skipping this field. | ||
| // CHECK-NEXT: def 3OverrideDescriptionOp | ||
|
|
||
| def 4OverrideExtraClassDeclarationOp : OpenMP_Op<"op", clauses=[OpenMP_SimpleClause]> { | ||
| let description = "Description of operation." # clausesDescription; | ||
| string extraClassDeclaration = ""; | ||
| } | ||
| // CHECK: warning: 'Simple' clause-defined `extraClassDeclaration` not present in operation. | ||
| // CHECK-SAME: Consider concatenating `clausesExtraClassDeclaration` or explicitly skipping this field. | ||
| // CHECK-NEXT: def 4OverrideExtraClassDeclarationOp | ||
|
|
||
|
|
||
| // ----------------------------------------------------------------------------- | ||
| // Verify that reporting is correct when OpenMP_Clause is inherited indirectly. | ||
| // ----------------------------------------------------------------------------- | ||
|
|
||
| class OpenMP_IndirectClauseSkip< | ||
| bit traits = false, bit arguments = false, bit assemblyFormat = false, | ||
| bit description = false, bit extraClassDeclaration = false | ||
| > : OpenMP_Clause</*isRequired=*/true, traits, arguments, assemblyFormat, | ||
| description, extraClassDeclaration> { | ||
| let arguments = (ins I32:$val2); | ||
| let assemblyFormat = "`val2` `(` $val2 `)`"; | ||
| let description = "Indirectly-inherited clause description."; | ||
| let extraClassDeclaration = "void indirectClauseExtraClassDecl();"; | ||
| } | ||
|
|
||
| def IndirectClause : OpenMP_IndirectClauseSkip<>; | ||
|
|
||
| def 5IndirectClauseOp : OpenMP_Op<"op", clauses=[IndirectClause]> { | ||
| let description = "Description of operation." # clausesDescription; | ||
| dag arguments = (ins I32:$myval); | ||
| } | ||
| // CHECK: warning: 'Indirect' clause-defined argument 'I32:$val2' not present in operation. | ||
| // CHECK-NEXT: def 5IndirectClauseOp | ||
|
|
||
|
|
||
| // ----------------------------------------------------------------------------- | ||
| // Verify that multiple clauses are taken into account. | ||
| // ----------------------------------------------------------------------------- | ||
|
|
||
| def 6MultiClauseOp : OpenMP_Op<"op", clauses=[OpenMP_SimpleClause, IndirectClause]> { | ||
| let description = "Description of operation." # clausesDescription; | ||
| let arguments = (ins I32:$val1); | ||
| let assemblyFormat = "`val2` `(` $val2 `)`"; | ||
| } | ||
| // CHECK: warning: 'Simple' clause-defined `assemblyFormat` not present in operation. | ||
| // CHECK-NEXT: def 6MultiClauseOp | ||
| // CHECK: warning: 'Indirect' clause-defined argument 'I32:$val2' not present in operation. | ||
| // CHECK-NEXT: def 6MultiClauseOp | ||
|
|
||
|
|
||
| // ----------------------------------------------------------------------------- | ||
| // Verify that reporting is correct when clause definitions have other | ||
| // superclasses in addition to OpenMP_Clause. | ||
| // ----------------------------------------------------------------------------- | ||
|
|
||
| class Placeholder {} | ||
| def MultiSuperClassClause : Placeholder, OpenMP_IndirectClauseSkip<>; | ||
|
|
||
| def 7MultiSuperClassClauseOp : OpenMP_Op<"op", clauses=[IndirectClause]> { | ||
| let description = "Description of operation." # clausesDescription; | ||
| dag arguments = (ins I32:$myval); | ||
| } | ||
| // CHECK: warning: 'Indirect' clause-defined argument 'I32:$val2' not present in operation. | ||
| // CHECK-NEXT: def 7MultiSuperClassClauseOp | ||
|
|
||
|
|
||
| // ----------------------------------------------------------------------------- | ||
| // Verify that no errors are produced if the field being overriden is also | ||
| // skipped for the clause. | ||
| // ----------------------------------------------------------------------------- | ||
|
|
||
| def SkipArgsOp : OpenMP_Op<"op", | ||
| clauses=[OpenMP_IndirectClauseSkip<arguments=true>]> { | ||
| let description = "Description of operation." # clausesDescription; | ||
| dag arguments = (ins I32:$myval); | ||
| } | ||
| def SkipAssemblyFormatOp : OpenMP_Op<"op", | ||
| clauses=[OpenMP_IndirectClauseSkip<assemblyFormat=true>]> { | ||
| let description = "Description of operation." # clausesDescription; | ||
| string assemblyFormat = "`alt_repr` `(` $val1 `)`"; | ||
| } | ||
| def SkipDescriptionOp : OpenMP_Op<"op", | ||
| clauses=[OpenMP_IndirectClauseSkip<description=true>]> { | ||
| let description = "Description of operation."; | ||
| } | ||
| def SkipExtraClassDeclarationOp : OpenMP_Op<"op", | ||
| clauses=[OpenMP_IndirectClauseSkip<extraClassDeclaration=true>]> { | ||
| let description = "Description of operation." # clausesDescription; | ||
| string extraClassDeclaration = ""; | ||
| } | ||
| // CHECK-NOT: error: | ||
| // CHECK-NOT: warning: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| //===- OmpOpGen.cpp - OpenMP dialect op specific generators ---------------===// | ||
| // | ||
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
| // See https://llvm.org/LICENSE.txt for license information. | ||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // OmpOpGen defines OpenMP dialect operation specific generators. | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #include "mlir/TableGen/GenInfo.h" | ||
|
|
||
| #include "llvm/TableGen/Error.h" | ||
| #include "llvm/TableGen/Record.h" | ||
|
|
||
| using namespace llvm; | ||
|
|
||
| /// Obtain the name of the OpenMP clause a given record inheriting | ||
| /// `OpenMP_Clause` refers to. | ||
| /// | ||
| /// It supports direct and indirect `OpenMP_Clause` superclasses. Once the | ||
| /// `OpenMP_Clause` class the record is based on is found, the optional | ||
| /// "OpenMP_" prefix and "Skip" and "Clause" suffixes are removed to return only | ||
| /// the clause name, i.e. "OpenMP_CollapseClauseSkip" is returned as "Collapse". | ||
| static StringRef extractOmpClauseName(Record *clause) { | ||
skatrak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Record *ompClause = clause->getRecords().getClass("OpenMP_Clause"); | ||
| assert(ompClause && "base OpenMP records expected to be defined"); | ||
|
|
||
| StringRef clauseClassName; | ||
| SmallVector<Record *, 1> clauseSuperClasses; | ||
| clause->getDirectSuperClasses(clauseSuperClasses); | ||
|
|
||
| // Check if OpenMP_Clause is a direct superclass. | ||
| for (Record *superClass : clauseSuperClasses) { | ||
| if (superClass == ompClause) { | ||
| clauseClassName = clause->getName(); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // Support indirectly-inherited OpenMP_Clauses. | ||
| if (clauseClassName.empty()) { | ||
| for (auto [superClass, _] : clause->getSuperClasses()) { | ||
| if (superClass->isSubClassOf(ompClause)) { | ||
| clauseClassName = superClass->getName(); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| assert(!clauseClassName.empty() && "clause name must be found"); | ||
|
|
||
| // Keep only the OpenMP clause name itself for reporting purposes. | ||
| StringRef prefix = "OpenMP_"; | ||
| StringRef suffixes[] = {"Skip", "Clause"}; | ||
|
|
||
| if (clauseClassName.starts_with(prefix)) | ||
| clauseClassName = clauseClassName.substr(prefix.size()); | ||
|
|
||
| for (StringRef suffix : suffixes) { | ||
| if (clauseClassName.ends_with(suffix)) | ||
| clauseClassName = | ||
| clauseClassName.substr(0, clauseClassName.size() - suffix.size()); | ||
| } | ||
|
|
||
| return clauseClassName; | ||
| } | ||
|
|
||
| /// Check that the given argument, identified by its name and initialization | ||
| /// value, is present in the \c arguments `dag`. | ||
| static bool verifyArgument(DagInit *arguments, StringRef argName, | ||
| Init *argInit) { | ||
| auto range = zip_equal(arguments->getArgNames(), arguments->getArgs()); | ||
| return std::find_if( | ||
| range.begin(), range.end(), | ||
| [&](std::tuple<llvm::StringInit *const &, llvm::Init *const &> v) { | ||
| return std::get<0>(v)->getAsUnquotedString() == argName && | ||
| std::get<1>(v) == argInit; | ||
| }) != range.end(); | ||
| } | ||
|
|
||
| /// Check that the given string record value, identified by its name \c value, | ||
| /// is either undefined or empty in both the given operation and clause record | ||
| /// or its contents for the clause record are contained in the operation record. | ||
| static bool verifyStringValue(StringRef value, Record *op, Record *clause) { | ||
| auto opValue = op->getValueAsOptionalString(value); | ||
| auto clauseValue = clause->getValueAsOptionalString(value); | ||
|
|
||
| bool opHasValue = opValue && !opValue->trim().empty(); | ||
| bool clauseHasValue = clauseValue && !clauseValue->trim().empty(); | ||
|
|
||
| if (!opHasValue) | ||
| return !clauseHasValue; | ||
|
|
||
| return !clauseHasValue || opValue->contains(clauseValue->trim()); | ||
| } | ||
|
|
||
| /// Verify that all fields of the given clause not explicitly ignored are | ||
| /// present in the corresponding operation field. | ||
| /// | ||
| /// Print warnings or errors where this is not the case. | ||
| static void verifyClause(Record *op, Record *clause) { | ||
| StringRef clauseClassName = extractOmpClauseName(clause); | ||
|
|
||
| if (!clause->getValueAsBit("ignoreArgs")) { | ||
| DagInit *opArguments = op->getValueAsDag("arguments"); | ||
| DagInit *arguments = clause->getValueAsDag("arguments"); | ||
|
|
||
| for (auto [name, arg] : | ||
| zip(arguments->getArgNames(), arguments->getArgs())) { | ||
| if (!verifyArgument(opArguments, name->getAsUnquotedString(), arg)) | ||
| PrintWarning( | ||
| op->getLoc(), | ||
| "'" + clauseClassName + "' clause-defined argument '" + | ||
| arg->getAsUnquotedString() + ":$" + | ||
| name->getAsUnquotedString() + | ||
| "' not present in operation. Consider `dag arguments = " | ||
| "!con(clausesArgs, ...)` or explicitly skipping this field."); | ||
| } | ||
| } | ||
|
|
||
| if (!clause->getValueAsBit("ignoreAsmFormat") && | ||
| !verifyStringValue("assemblyFormat", op, clause)) | ||
| PrintWarning( | ||
| op->getLoc(), | ||
| "'" + clauseClassName + | ||
| "' clause-defined `assemblyFormat` not present in operation. " | ||
| "Consider concatenating `clausesAssemblyFormat` or explicitly " | ||
| "skipping this field."); | ||
|
|
||
| if (!clause->getValueAsBit("ignoreDesc") && | ||
| !verifyStringValue("description", op, clause)) | ||
| PrintError(op->getLoc(), | ||
| "'" + clauseClassName + | ||
| "' clause-defined `description` not present in operation. " | ||
| "Consider concatenating `clausesDescription` or explicitly " | ||
| "skipping this field."); | ||
|
|
||
| if (!clause->getValueAsBit("ignoreExtraDecl") && | ||
| !verifyStringValue("extraClassDeclaration", op, clause)) | ||
| PrintWarning( | ||
| op->getLoc(), | ||
| "'" + clauseClassName + | ||
| "' clause-defined `extraClassDeclaration` not present in " | ||
| "operation. Consider concatenating `clausesExtraClassDeclaration` " | ||
| "or explicitly skipping this field."); | ||
| } | ||
|
|
||
| /// Verify that all properties of `OpenMP_Clause`s of records deriving from | ||
| /// `OpenMP_Op`s have been inherited by the latter. | ||
| static bool verifyDecls(const RecordKeeper &recordKeeper, raw_ostream &) { | ||
| for (Record *op : recordKeeper.getAllDerivedDefinitions("OpenMP_Op")) { | ||
| for (Record *clause : op->getValueAsListOfDefs("clauseList")) | ||
| verifyClause(op, clause); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| // Registers the generator to mlir-tblgen. | ||
| static mlir::GenRegistration | ||
| verifyOpenmpOps("verify-openmp-ops", | ||
| "Verify OpenMP operations (produce no output file)", | ||
| verifyDecls); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, will this generate a file called
no-output, which will further me included into theMlirOpenMPOpsIncGentarget below?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the quick review! I suppose it might do, though if that's the case it will always be empty. I'm not very familiar with how this infrastructure works, so I don't know if that could cause an issue. After a quick search I can't find a definition for
add_public_tablegen_targetto check this either 🤔.Edit: Found it now, will check and get back if I can think of a better solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made some changes here that basically remove the empty "no-output" files from the build directory and also prevent it from being added as a dependency. It's admittedly a bit of a hack, but I don't know if it's worth the effort to refactor the
mlir_tablegenandtablegenCMake functions to allow non-generators like this one, since it seems like something that won't be useful that often.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for investigating! It's not a big issue if the file is still present, but better avoid it in dependencies. In case you get complains about windows builds where paths are different, feel free to drop the file removal and only keep the variable manipulation.