[Mlir-commits] [mlir] 0f827ee - [mlir][ods] add mechanism for deprecating an `OpBuilder` in ODS

Markus Böck llvmlistbot at llvm.org
Tue Feb 7 07:49:33 PST 2023


Author: Markus Böck
Date: 2023-02-07T16:49:45+01:00
New Revision: 0f827ee0366922c16fe987c75dad459e717b4f6e

URL: https://github.com/llvm/llvm-project/commit/0f827ee0366922c16fe987c75dad459e717b4f6e
DIFF: https://github.com/llvm/llvm-project/commit/0f827ee0366922c16fe987c75dad459e717b4f6e.diff

LOG: [mlir][ods] add mechanism for deprecating an `OpBuilder` in ODS

This allows discouraging the use of specific build methods of an op by having TableGen generate C++ code, instructing the C++ compiler to warn on use of the `build` method.
The implementation uses the C++14 `[[deprecated(...)]]`` for this purpose. I considered using `LLVM_DEPRECATED`, but thought requiring a fix-it was not necassery, nor would the syntax in ODS have been very nice.

My motivation for this change is that in the future I'd like to deprecate the use `build` methods in the LLVM Dialect, not using explicit pointer types for ops such as `llvm.load` or `llvm.alloca`, which makes the code not future proof for opaque pointers. In-tree has to be clean first before I could commit such a change of course, but I thought the initial infrastructure change could already be submitted.

Differential Revision: https://reviews.llvm.org/D143190

Added: 
    

Modified: 
    mlir/docs/DefiningDialects/Operations.md
    mlir/include/mlir/IR/DialectBase.td
    mlir/include/mlir/IR/OpBase.td
    mlir/include/mlir/TableGen/Builder.h
    mlir/include/mlir/TableGen/Class.h
    mlir/lib/TableGen/Builder.cpp
    mlir/lib/TableGen/Class.cpp
    mlir/test/mlir-tblgen/op-decl-and-defs.td
    mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/docs/DefiningDialects/Operations.md b/mlir/docs/DefiningDialects/Operations.md
index 03a2adbfcfd5..b904ed7d8a75 100644
--- a/mlir/docs/DefiningDialects/Operations.md
+++ b/mlir/docs/DefiningDialects/Operations.md
@@ -1571,7 +1571,7 @@ mlir-tblgen --gen-op-interface-doc -I /path/to/mlir/include /path/to/input/td/fi
 
 ## Appendix
 
-### Reporting deprecation
+### Reporting deprecation in TableGen
 
 Classes/defs can be marked as deprecated by using the `Deprecate` helper class,
 e.g.,
@@ -1584,6 +1584,29 @@ would result in marking `OpTraitA` as deprecated and mlir-tblgen can emit a
 warning (default) or error (depending on `-on-deprecated` flag) to make
 deprecated state known.
 
+### Reporting deprecation in C++
+
+TableGen generated C++ entities, such as classes, functions or methods, can be
+marked as deprecated using the `CppDeprecated` mixin:
+
+```tablegen
+def MyOp : Op<MyDialect, "my.op">, CppDeprecated<"use 'your.op' instead">;
+```
+
+This 
diff ers to the deprecation mechanic for TableGen, in that no warning is
+emitted by mlir-tblgen. Rather, a warning with the given reason is emitted by
+the C++ compiler on use of the given entity.
+
+To allow more convenient syntax, helper classes exist for TableGen classes
+which are commonly used as anonymous definitions. These currently include:
+
+* `DeprecatedOpBuilder`: Can be used in place of `OpBuilder` with the same
+  arguments except taking the reason as first argument, e.g. 
+  `DeprecatedOpBuilder<"use 'build' with foo instead", (ins "int":$bar)>`
+
+Note: Support for the `CppDeprecated` mechanism has to be implemented by 
+every code generator separately. 
+
 ### Requirements and existing mechanisms analysis
 
 The op description should be as declarative as possible to allow a wide range of

diff  --git a/mlir/include/mlir/IR/DialectBase.td b/mlir/include/mlir/IR/DialectBase.td
index 27b988d632e2..4b8e6c6e3b46 100644
--- a/mlir/include/mlir/IR/DialectBase.td
+++ b/mlir/include/mlir/IR/DialectBase.td
@@ -13,12 +13,24 @@
 #ifndef DIALECTBASE_TD
 #define DIALECTBASE_TD
 
-// Helper for marking deprecated classes or defs. To mark a def as deprecated,
-// mix in the `Deprecate` class with a reason.
+// Helper for marking deprecated classes or defs in TableGen. To mark a def as
+// deprecated, mix in the `Deprecate` class with a reason.
+// Usage of a deprecated def within TableGen will cause a warning with the
+// given message.
 class Deprecated<string reason> {
   string odsDeprecated = reason;
 }
 
+// Helper for marking entities in ODS generated C++ as deprecated.
+// Usage of such an entity from C++ code will cause a warning being emitted by
+// the C++ compiler with the given message.
+//
+// Note: Support has to be implemented by the code generator of a given
+// entity.
+class CppDeprecated<string reason> {
+  string odsCppDeprecated = reason;
+}
+
 //===----------------------------------------------------------------------===//
 // Dialect definitions
 //===----------------------------------------------------------------------===//

diff  --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td
index e833b0e3aec4..cd888ac61f4d 100644
--- a/mlir/include/mlir/IR/OpBase.td
+++ b/mlir/include/mlir/IR/OpBase.td
@@ -2235,6 +2235,12 @@ class OpBuilder<dag p, code b = ""> {
   code body = b;
 }
 
+// OpBuilder like the above, but the emitted 'build' method is marked as
+// deprecated in C++. Use of it will emit a warning by the C++ compiler
+// with the given reason.
+class DeprecatedOpBuilder<string reason, dag p, code b = "">
+  : OpBuilder<p, b>, CppDeprecated<reason>;
+
 // A base decorator class that may optionally be added to OpVariables.
 class OpVariableDecorator;
 

diff  --git a/mlir/include/mlir/TableGen/Builder.h b/mlir/include/mlir/TableGen/Builder.h
index d6e34b79a50f..a733bc569fb3 100644
--- a/mlir/include/mlir/TableGen/Builder.h
+++ b/mlir/include/mlir/TableGen/Builder.h
@@ -70,6 +70,10 @@ class Builder {
   /// Return an optional string containing the body of the builder.
   std::optional<StringRef> getBody() const;
 
+  /// Return the deprecation message of the builder.
+  /// Empty optional if the builder is not deprecated.
+  std::optional<StringRef> getDeprecatedMessage() const;
+
 protected:
   /// The TableGen definition of this builder.
   const llvm::Record *def;

diff  --git a/mlir/include/mlir/TableGen/Class.h b/mlir/include/mlir/TableGen/Class.h
index 9c4efcd41cd6..10dfc01961a0 100644
--- a/mlir/include/mlir/TableGen/Class.h
+++ b/mlir/include/mlir/TableGen/Class.h
@@ -329,6 +329,11 @@ class Method : public ClassDeclarationBase<ClassDeclaration::Method> {
   /// Get the method body.
   MethodBody &body() { return methodBody; }
 
+  /// Sets or removes the deprecation message of the method.
+  void setDeprecated(std::optional<StringRef> message) {
+    this->deprecationMessage = message;
+  }
+
   /// Returns true if this is a static method.
   bool isStatic() const { return properties & Static; }
 
@@ -369,6 +374,8 @@ class Method : public ClassDeclarationBase<ClassDeclaration::Method> {
   MethodSignature methodSignature;
   /// The body of the method, if it has one.
   MethodBody methodBody;
+  /// Deprecation message if the method is deprecated.
+  std::optional<std::string> deprecationMessage;
 };
 
 /// This enum describes C++ inheritance visibility.

diff  --git a/mlir/lib/TableGen/Builder.cpp b/mlir/lib/TableGen/Builder.cpp
index 48fb5065f4bd..47a2f6cc4456 100644
--- a/mlir/lib/TableGen/Builder.cpp
+++ b/mlir/lib/TableGen/Builder.cpp
@@ -81,3 +81,9 @@ std::optional<StringRef> Builder::getBody() const {
   std::optional<StringRef> body = def->getValueAsOptionalString("body");
   return body && !body->empty() ? body : std::nullopt;
 }
+
+std::optional<StringRef> Builder::getDeprecatedMessage() const {
+  std::optional<StringRef> message =
+      def->getValueAsOptionalString("odsCppDeprecated");
+  return message && !message->empty() ? message : std::nullopt;
+}

diff  --git a/mlir/lib/TableGen/Class.cpp b/mlir/lib/TableGen/Class.cpp
index c562cfde0a9c..fd2ba65e568e 100644
--- a/mlir/lib/TableGen/Class.cpp
+++ b/mlir/lib/TableGen/Class.cpp
@@ -114,6 +114,11 @@ void MethodBody::writeTo(raw_indented_ostream &os) const {
 //===----------------------------------------------------------------------===//
 
 void Method::writeDeclTo(raw_indented_ostream &os) const {
+  if (deprecationMessage) {
+    os << "[[deprecated(\"";
+    os.write_escaped(*deprecationMessage);
+    os << "\")]]\n";
+  }
   if (isStatic())
     os << "static ";
   if (properties & ConstexprValue)

diff  --git a/mlir/test/mlir-tblgen/op-decl-and-defs.td b/mlir/test/mlir-tblgen/op-decl-and-defs.td
index dd34f706d35a..3a486a12bb01 100644
--- a/mlir/test/mlir-tblgen/op-decl-and-defs.td
+++ b/mlir/test/mlir-tblgen/op-decl-and-defs.td
@@ -38,7 +38,9 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
     VariadicRegion<AnyRegion>:$someRegions
   );
   let builders = [OpBuilder<(ins "Value":$val)>,
-                  OpBuilder<(ins CArg<"int", "0">:$integer)>];
+                  OpBuilder<(ins CArg<"int", "0">:$integer)>,
+                  DeprecatedOpBuilder<"the deprecation message",
+                            (ins "float":$something)>];
   let hasCustomAssemblyFormat = 1;
 
   let hasCanonicalizer = 1;
@@ -107,6 +109,8 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
 // CHECK:   ::mlir::Attribute removeSomeAttr2Attr();
 // CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, Value val);
 // CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, int integer = 0);
+// CHECK{LITERAL}:   [[deprecated("the deprecation message")]]
+// CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, float something);
 // CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::Type r, ::mlir::TypeRange s, ::mlir::Value a, ::mlir::ValueRange b, ::mlir::IntegerAttr attr1, /*optional*/::mlir::FloatAttr some_attr2, unsigned someRegionsCount)
 // CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::Value a, ::mlir::ValueRange b, ::mlir::IntegerAttr attr1, /*optional*/::mlir::FloatAttr some_attr2, unsigned someRegionsCount);
 // CHECK:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::Type r, ::mlir::TypeRange s, ::mlir::Value a, ::mlir::ValueRange b, uint32_t attr1, /*optional*/::mlir::FloatAttr some_attr2, unsigned someRegionsCount)

diff  --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index bc3e2599e72d..ef430bfe1b05 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -1965,6 +1965,9 @@ void OpEmitter::genBuilder() {
     if (body)
       ERROR_IF_PRUNED(method, "build", op);
 
+    if (method)
+      method->setDeprecated(builder.getDeprecatedMessage());
+
     FmtContext fctx;
     fctx.withBuilder(odsBuilder);
     fctx.addSubst("_state", builderOpState);


        


More information about the Mlir-commits mailing list