[llvm-branch-commits] [mlir] 2f5569f - [mlir] remove deprecated string-based OpBuilder from ODS

Alex Zinenko via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Dec 22 01:02:21 PST 2020


Author: Alex Zinenko
Date: 2020-12-22T09:57:49+01:00
New Revision: 2f5569f6f67a30f7774f7c2d2f3d726752a862ae

URL: https://github.com/llvm/llvm-project/commit/2f5569f6f67a30f7774f7c2d2f3d726752a862ae
DIFF: https://github.com/llvm/llvm-project/commit/2f5569f6f67a30f7774f7c2d2f3d726752a862ae.diff

LOG: [mlir] remove deprecated string-based OpBuilder from ODS

It has been deprecated with a warning for two months, removing.

Reviewed By: mehdi_amini

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

Added: 
    

Modified: 
    mlir/include/mlir/IR/OpBase.td
    mlir/test/mlir-tblgen/op-decl.td
    mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td
index 0f060b2b1a0a..0ae572c38f49 100644
--- a/mlir/include/mlir/IR/OpBase.td
+++ b/mlir/include/mlir/IR/OpBase.td
@@ -1939,15 +1939,6 @@ def region;
 // Marker used to identify the successor list for an op.
 def successor;
 
-// Base class for custom builders. This is a transient class that will go away
-// when the transition to the DAG form of builder declaration is complete.
-// Should not be used directly.
-class OpBuilderBase<dag dp, code b> {
-  string params = ?;
-  dag dagParams = dp;
-  code body = b;
-}
-
 // Class for defining a custom builder.
 //
 // TableGen generates several generic builders for each op by default (see
@@ -1986,11 +1977,9 @@ class OpBuilderBase<dag dp, code b> {
 // If an empty string is passed in for `body`, then *only* the builder
 // declaration will be generated; this provides a way to define complicated
 // builders entirely in C++.
-class OpBuilderDAG<dag p, code b = ""> : OpBuilderBase<p, b>;
-
-// Deprecated version of OpBuilder that takes the builder signature as string.
-class OpBuilder<string p, code b = ""> : OpBuilderBase<(ins), b> {
-  let params = p;
+class OpBuilderDAG<dag p, code b = ""> {
+  dag dagParams = p;
+  code body = b;
 }
 
 // A base decorator class that may optionally be added to OpVariables.
@@ -2068,7 +2057,7 @@ class Op<Dialect dialect, string mnemonic, list<OpTrait> props = []> {
   //                   ValueRange operands,
   //                   ArrayRef<NamedAttribute> attributes);
   // ```
-  list<OpBuilderBase> builders = ?;
+  list<OpBuilderDAG> builders = ?;
 
   // Avoid generating default build functions.  Custom builders must be
   // provided.

diff  --git a/mlir/test/mlir-tblgen/op-decl.td b/mlir/test/mlir-tblgen/op-decl.td
index 29438f1836a7..13daca67c475 100644
--- a/mlir/test/mlir-tblgen/op-decl.td
+++ b/mlir/test/mlir-tblgen/op-decl.td
@@ -34,8 +34,7 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
     VariadicRegion<AnyRegion>:$someRegions
   );
   let builders = [OpBuilderDAG<(ins "Value":$val)>,
-                  OpBuilderDAG<(ins CArg<"int", "0">:$integer)>,
-                  OpBuilder<"double deprecatedForm">];
+                  OpBuilderDAG<(ins CArg<"int", "0">:$integer)>];
   let parser = [{ foo }];
   let printer = [{ bar }];
   let verifier = [{ baz }];
@@ -84,7 +83,6 @@ def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
 // CHECK:   ::llvm::Optional< ::llvm::APFloat > attr2();
 // 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:   static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, double deprecatedForm);
 // 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 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 attr2, unsigned someRegionsCount)
 // CHECK:   static void build(::mlir::OpBuilder &, ::mlir::OperationState &odsState, ::mlir::TypeRange resultTypes, ::mlir::ValueRange operands, ::llvm::ArrayRef<::mlir::NamedAttribute> attributes, unsigned numRegions)

diff  --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 1c8cbfb9db38..40e1c355daf8 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -1304,8 +1304,7 @@ void OpEmitter::genUseAttrAsResultTypeBuilder() {
 /// Updates the context `fctx` to enable replacement of $_builder and $_state
 /// in the body. Reports errors at `loc`.
 static std::string builderSignatureFromDAG(const DagInit *init,
-                                           ArrayRef<llvm::SMLoc> loc,
-                                           FmtContext &fctx) {
+                                           ArrayRef<llvm::SMLoc> loc) {
   auto *defInit = dyn_cast<DefInit>(init->getOperator());
   if (!defInit || !defInit->getDef()->getName().equals("ins"))
     PrintFatalError(loc, "expected 'ins' in builders");
@@ -1351,31 +1350,9 @@ static std::string builderSignatureFromDAG(const DagInit *init,
         llvm::formatv("{0} {1}{2}", type, name, defaultValue).str());
   }
 
-  fctx.withBuilder(builder);
-  fctx.addSubst("_state", builderOpState);
-
   return llvm::join(arguments, ", ");
 }
 
-// Returns a signature fo the builder as defined by a string initializer,
-// optionally injecting the builder and state arguments.
-// TODO: to be removed after the transition is complete.
-static std::string builderSignatureFromString(StringRef params,
-                                              FmtContext &fctx) {
-  bool skipParamGen = params.startswith("OpBuilder") ||
-                      params.startswith("mlir::OpBuilder") ||
-                      params.startswith("::mlir::OpBuilder");
-  if (skipParamGen)
-    return params.str();
-
-  fctx.withBuilder(builder);
-  fctx.addSubst("_state", builderOpState);
-  return std::string(llvm::formatv("::mlir::OpBuilder &{0}, "
-                                   "::mlir::OperationState &{1}{2}{3}",
-                                   builder, builderOpState,
-                                   params.empty() ? "" : ", ", params));
-}
-
 void OpEmitter::genBuilder() {
   // Handle custom builders if provided.
   // TODO: Create wrapper class for OpBuilder to hide the native
@@ -1385,19 +1362,8 @@ void OpEmitter::genBuilder() {
     if (listInit) {
       for (Init *init : listInit->getValues()) {
         Record *builderDef = cast<DefInit>(init)->getDef();
-        llvm::Optional<StringRef> params =
-            builderDef->getValueAsOptionalString("params");
-        FmtContext fctx;
-        if (params.hasValue()) {
-          PrintWarning(op.getLoc(),
-                       "Op uses a deprecated, string-based OpBuilder format; "
-                       "use OpBuilderDAG with '(ins <...>)' instead");
-        }
-        std::string paramStr =
-            params.hasValue() ? builderSignatureFromString(params->trim(), fctx)
-                              : builderSignatureFromDAG(
-                                    builderDef->getValueAsDag("dagParams"),
-                                    op.getLoc(), fctx);
+        std::string paramStr = builderSignatureFromDAG(
+            builderDef->getValueAsDag("dagParams"), op.getLoc());
 
         StringRef body = builderDef->getValueAsString("body");
         bool hasBody = !body.empty();
@@ -1405,6 +1371,10 @@ void OpEmitter::genBuilder() {
             hasBody ? OpMethod::MP_Static : OpMethod::MP_StaticDeclaration;
         auto *method =
             opClass.addMethodAndPrune("void", "build", properties, paramStr);
+
+        FmtContext fctx;
+        fctx.withBuilder(builder);
+        fctx.addSubst("_state", builderOpState);
         if (hasBody)
           method->body() << tgfmt(body, &fctx);
       }


        


More information about the llvm-branch-commits mailing list