[tblgen] Add command line flag for using fallback TypeIDs in generation #125767
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-core Author: Stef Lindall (bethebunny) ChangesAdds two new CL flags (I didn't find a simple way to just add one):
When specified, these modify type, attr, and op decl+definition generation to not emit Full diff: https://github.com/llvm/llvm-project/pull/125767.diff 2 Files Affected:
diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
index 6a39424bd463fd7..5134ccac8671446 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -4,6 +4,7 @@
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
+//
//===----------------------------------------------------------------------===//
#include "AttrOrTypeFormatGen.h"
@@ -25,6 +26,16 @@ using namespace mlir::tblgen;
using llvm::Record;
using llvm::RecordKeeper;
+//===----------------------------------------------------------------------===//
+// Utility structs and functions
+//===----------------------------------------------------------------------===//
+
+static llvm::cl::OptionCategory clAttrOrTypeDefs("Options for -gen-*def-*");
+
+static llvm::cl::opt<bool> clUseFallbackTypeIDs(
+ "gen-attr-or-type-use-fallback-type-ids", llvm::cl::desc("Don't generate static TypeID decls; fall back to string comparison."),
+ llvm::cl::init(false), llvm::cl::cat(clAttrOrTypeDefs));
+
//===----------------------------------------------------------------------===//
// Utility Functions
//===----------------------------------------------------------------------===//
@@ -773,7 +784,7 @@ bool DefGenerator::emitDecls(StringRef selectedDialect) {
// Emit the TypeID explicit specializations to have a single definition for
// each of these.
for (const AttrOrTypeDef &def : defs)
- if (!def.getDialect().getCppNamespace().empty())
+ if (!clUseFallbackTypeIDs && !def.getDialect().getCppNamespace().empty())
os << "MLIR_DECLARE_EXPLICIT_TYPE_ID("
<< def.getDialect().getCppNamespace() << "::" << def.getCppClassName()
<< ")\n";
@@ -986,7 +997,7 @@ bool DefGenerator::emitDefs(StringRef selectedDialect) {
gen.emitDef(os);
}
// Emit the TypeID explicit specializations to have a single symbol def.
- if (!def.getDialect().getCppNamespace().empty())
+ if (!clUseFallbackTypeIDs && !def.getDialect().getCppNamespace().empty())
os << "MLIR_DEFINE_EXPLICIT_TYPE_ID("
<< def.getDialect().getCppNamespace() << "::" << def.getCppClassName()
<< ")\n";
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index a970cbc5cacebe3..e3c87e1e578ebdc 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -31,6 +31,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/Signals.h"
@@ -233,6 +234,16 @@ static const char *const opCommentHeader = R"(
// Utility structs and functions
//===----------------------------------------------------------------------===//
+static llvm::cl::OptionCategory clOpDefs("Options for op definitions");
+
+static llvm::cl::opt<bool> clUseFallbackTypeIDs(
+ "gen-op-use-fallback-type-ids", llvm::cl::desc("Don't generate static TypeID decls; fall back to string comparison."),
+ llvm::cl::init(false), llvm::cl::cat(clOpDefs));
+
+//===----------------------------------------------------------------------===//
+// Utility structs and functions
+//===----------------------------------------------------------------------===//
+
// Replaces all occurrences of `match` in `str` with `substitute`.
static std::string replaceAllSubstrs(std::string str, const std::string &match,
const std::string &substitute) {
@@ -4625,7 +4636,7 @@ emitOpClasses(const RecordKeeper &records,
OpEmitter::emitDecl(op, os, staticVerifierEmitter);
}
// Emit the TypeID explicit specialization to have a single definition.
- if (!op.getCppNamespace().empty())
+ if (!clUseFallbackTypeIDs && !op.getCppNamespace().empty())
os << "MLIR_DECLARE_EXPLICIT_TYPE_ID(" << op.getCppNamespace()
<< "::" << op.getCppClassName() << ")\n\n";
} else {
@@ -4636,7 +4647,7 @@ emitOpClasses(const RecordKeeper &records,
OpEmitter::emitDef(op, os, staticVerifierEmitter);
}
// Emit the TypeID explicit specialization to have a single definition.
- if (!op.getCppNamespace().empty())
+ if (!clUseFallbackTypeIDs && !op.getCppNamespace().empty())
os << "MLIR_DEFINE_EXPLICIT_TYPE_ID(" << op.getCppNamespace()
<< "::" << op.getCppClassName() << ")\n\n";
}
|
You can test this locally with the following command:git-clang-format --diff 2a84e1e65ad7f84c2dbcf37241a7d1805a523e0d 5379ad651f9f82f8d35374fa4c46f4a511eedc88 --extensions cpp,h -- mlir/tools/mlir-tblgen/SharedCL.cpp mlir/tools/mlir-tblgen/SharedCL.h mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp mlir/tools/mlir-tblgen/DialectGen.cpp mlir/tools/mlir-tblgen/OpDefinitionsGen.cppView the diff from clang-format here.diff --git a/mlir/tools/mlir-tblgen/DialectGen.cpp b/mlir/tools/mlir-tblgen/DialectGen.cpp
index e230100f34..bd5a89f56d 100644
--- a/mlir/tools/mlir-tblgen/DialectGen.cpp
+++ b/mlir/tools/mlir-tblgen/DialectGen.cpp
@@ -10,8 +10,8 @@
//
//===----------------------------------------------------------------------===//
-#include "SharedCL.h"
#include "DialectGenUtilities.h"
+#include "SharedCL.h"
#include "mlir/TableGen/Class.h"
#include "mlir/TableGen/CodeGenHelpers.h"
#include "mlir/TableGen/Format.h"
|
bethebunny
changed the title
|
That's an interesting direction! |
|
Huh I hadn't thought of that! Before updating the PR I wanted to make sure I'm understanding your suggestion. Is this kindof what you had in mind? https://gist.github.com/bethebunny/a12b4e021029b02ce822520ca62e3bc0 This isn't specifically in the macro definition, but adds an additional |
|
This is what I had in mind indeed (I would think you'd need to guard line 52 as well?). Now the comment line 29 seems like a hurdle:
In general I'm unsure about the risk of name conflicts with this? |
I think this is OK. Types in anonymous contexts shouldn't be able to be shared across DSOs anyway, so it's fine for them to continue to define as self-owned. |
|
Sent as a separate PR here: #126999 |
Adds a new
-use-fallback-type-idsflag to tblgen.When specified, this flag causes type, attr, op, and dialect decl+definition generation to not emit
MLIR_*_EXPLICIT_TYPE_IDmacros. This falls back to string TypeIDs which use string comparison, and a potential avenue in avoiding complications related to https://discourse.llvm.org/t/more-work-needed-for-typeid-duplication-help-wanted/4041/14.