[Mlir-commits] [mlir] f117bbc - [mlir] Add opt-in default property bytecode read and write implementation

[Markus Böck]

Author: Markus Böck
Date: 2023-07-21T08:03:26+02:00
New Revision: f117bbca041c9b0c59002835d939e1bc9e76cf39

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

LOG: [mlir] Add opt-in default property bytecode read and write implementation

Using properties currently requires at the very least implementing four methods/code snippets:
* `convertToAttribute`
* `convertFromAttribute`
* `writeToMlirBytecode`
* `readFromMlirBytecode`

This makes replacing attributes with properties harder than it has to be: Attributes by default do not require immediately defining custom bytecode encoding.

This patch therefore adds opt-in implementations of `writeToMlirBytecode` and `readFromMlirBytecode` that work with the default implementations of `convertToAttribute` and `convertFromAttribute`. They are provided by `defvar`s in `OpBase.td` and can be used by adding:
```
let writeToMlirBytecode = writeMlirBytecodeWithConvertToAttribute;
let readFromMlirBytecode = readMlirBytecodeUsingConvertFromAttribute;
```
to ones TableGen definition.

While this bytecode encoding is almost certainly not ideal for a given property, it allows more incremental use of properties and getting something sane working before optimizing the bytecode format.

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

Added: 
    mlir/test/IR/properties-bytecode-roundtrip.mlir

Modified: 
    mlir/include/mlir/IR/OpBase.td
    mlir/test/lib/Dialect/Test/TestOps.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 28a1716228e8f0..940588b7c0f9f2 100644
--- a/mlir/include/mlir/IR/OpBase.td
+++ b/mlir/include/mlir/IR/OpBase.td
@@ -243,6 +243,7 @@ class Property<string storageTypeParam = "", string desc = ""> {
   // Format:
   // - `$_storage` is the storage type value.
   // - `$_writer` is a `DialectBytecodeWriter`.
+  // - `$_ctxt` is a `MLIRContext *`.
   code writeToMlirBytecode = [{
     writeToMlirBytecode($_writer, $_storage)
   }];
@@ -252,6 +253,7 @@ class Property<string storageTypeParam = "", string desc = ""> {
   // Format:
   // - `$_storage` is the storage type value.
   // - `$_reader` is a `DialectBytecodeReader`.
+  // - `$_ctxt` is a `MLIRContext *`.
   code readFromMlirBytecode = [{
     if (::mlir::failed(readFromMlirBytecode($_reader, $_storage)))
       return ::mlir::failure();
@@ -261,6 +263,38 @@ class Property<string storageTypeParam = "", string desc = ""> {
   string defaultValue = ?;
 }
 
+/// Implementation of the Property class's `readFromMlirBytecode` field using
+/// the default `convertFromAttribute` implementation.
+/// Users not wanting to implement their own `readFromMlirBytecode` and
+/// `writeToMlirBytecode` implementations can opt into using this implementation
+/// by writing:
+///
+/// let writeToMlirBytecode = writeMlirBytecodeWithConvertToAttribute;
+/// let readFromMlirBytecode = readMlirBytecodeUsingConvertFromAttribute;
+///
+/// in their property definition.
+/// Serialization and deserialization is performed using the attributes
+/// returned by `convertFromAttribute` and `convertToAttribute`.
+///
+/// WARNING: This implementation creates a less than optimal encoding.
+/// Users caring about optimal encoding should not use this implementation and
+/// implement `readFromMlirBytecode` and `writeToMlirBytecode` themselves.
+defvar readMlirBytecodeUsingConvertFromAttribute = [{
+  ::mlir::Attribute attr;
+  if (::mlir::failed($_reader.readAttribute(attr)))
+    return ::mlir::failure();
+  if (::mlir::failed(convertFromAttribute($_storage, attr, nullptr)))
+    return ::mlir::failure();
+}];
+
+/// Implementation of the Property class's `writeToMlirBytecode` field using
+/// the default `convertToAttribute` implementation.
+/// See description of `readMlirBytecodeUsingConvertFromAttribute` above for
+/// details.
+defvar writeMlirBytecodeWithConvertToAttribute = [{
+  $_writer.writeAttribute(convertToAttribute($_ctxt, $_storage))
+}];
+
 // Subclass for constraints on an attribute.
 class AttrConstraint<Pred predicate, string summary = ""> :
     Constraint<predicate, summary>;

diff  --git a/mlir/test/IR/properties-bytecode-roundtrip.mlir b/mlir/test/IR/properties-bytecode-roundtrip.mlir
new file mode 100644
index 00000000000000..b4c36430554254
--- /dev/null
+++ b/mlir/test/IR/properties-bytecode-roundtrip.mlir
@@ -0,0 +1,5 @@
+// RUN: mlir-opt -emit-bytecode %s | mlir-opt | FileCheck %s
+
+// CHECK-LABEL: "test.using_int_property_with_worse_bytecode"
+// CHECK-SAME: value = 3
+"test.using_int_property_with_worse_bytecode"() <{value = 3}> : () -> ()

diff  --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 389fac6f3fed6f..6cb47451ec8b70 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -3367,6 +3367,17 @@ def TestOpUsingPropertyRefInCustom : TEST_Op<"using_property_ref_in_custom"> {
   let arguments = (ins IntProperty<"int64_t">:$first, IntProperty<"int64_t">:$second);
 }
 
+def IntPropertyWithWorseBytecode : Property<"int64_t"> {
+  let writeToMlirBytecode = writeMlirBytecodeWithConvertToAttribute;
+
+  let readFromMlirBytecode = readMlirBytecodeUsingConvertFromAttribute;
+}
+
+def TestOpUsingIntPropertyWithWorseBytecode
+    : TEST_Op<"using_int_property_with_worse_bytecode"> {
+  let arguments = (ins IntPropertyWithWorseBytecode:$value);
+}
+
 // Op with a properties struct defined out-of-line. The struct has custom
 // printer/parser.
 

diff  --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index a935e21152b910..c069e7ce89a110 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -1360,7 +1360,8 @@ void OpEmitter::genPropertiesSupport() {
       FmtContext fctx;
       fctx.addSubst("_reader", "reader")
           .addSubst("_writer", "writer")
-          .addSubst("_storage", propertyStorage);
+          .addSubst("_storage", propertyStorage)
+          .addSubst("_ctxt", "this->getContext()");
       readPropertiesMethod << formatv(
           R"(
   {{