[mlir] Add default property bytecode read and write implementation

[zero9178]

Summary

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 defvars 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.

[mehdi_amini]

This makes it too easy to be "misused": that is people will think they have support for properties without knowing they don't have bytecode support: can we do something better here?
Right now the idea was that it's under control of the user by defining their ODS subclass of Property to override writeToMlirBytecode / readFromMlirBytecode, where they can call the attribute conversion if they like.

[zero9178]

In D155286#4501592, @mehdi_amini wrote:

This makes it too easy to be "misused": that is people will think they have support for properties without knowing they don't have bytecode support: can we do something better here?
Right now the idea was that it's under control of the user by defining their ODS subclass of Property to override writeToMlirBytecode / readFromMlirBytecode, where they can call the attribute conversion if they like.

I see where you're coming from but I disagree with the wording. People do have support for bytecode here in the sense that it makes it possible to serialize their properties to and from bytecode. The experience is essentially the same as with attributes without a corresponding BytecodeDialectInterface implementation in that it'll just work. The "misuse" is simply that the encoding is suboptimal in size and speed.

This patch does also not take away the choice from the user, it just tries to be a more comfortable default.

I suppose an alternative to the above would be to have a defvar with the default so that users can manually say let writeToMlirBytecode = useConvertToAttribute; for example.

[mehdi_amini]

I suppose an alternative to the above would be to have a defvar with the default so that users can manually say let writeToMlirBytecode = useConvertToAttribute; for example.

Right, I'd rather see this I think.

[zero9178]

Make the new default opt-in and an almost pure TableGen implementation.
This new implementation has the only disadvantage compared to the previous one in that it only works with the default implementations of convertToAttribute and convertFromAttribute.

mlir/include/mlir/IR/OpBase.td

code hashProperty = [{
     llvm::hash_value($_storage);
   }];
 
   // The call expression to emit the storage type to bytecode.
   //
   // Format:
   // - `$_storage` is the storage type value.
   // - `$_writer` is a `DialectBytecodeWriter`.
+  // - `$_ctxt` is a `MLIRContext *`.
   code writeToMlirBytecode = [{
     writeToMlirBytecode($_writer, $_storage)
   }];
 
   // The call expression to read the storage type from bytecode.
   //
   // 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();
   }];
 
   // Default value for the property.
   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>;
 
 // Subclass for constraints on a region.
 class RegionConstraint<Pred predicate, string summary = ""> :
     Constraint<predicate, summary>;
 

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

+// 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}> : () -> ()

mlir/test/lib/Dialect/Test/TestOps.td

def TestOpUsingPropertyInCustom : TEST_Op<"using_property_in_custom"> {
   let arguments = (ins ArrayProperty<"int64_t", 3>:$prop);
 }
 
 def TestOpUsingPropertyRefInCustom : TEST_Op<"using_property_ref_in_custom"> {
   let assemblyFormat = "custom<IntProperty>($first) `+` custom<SumProperty>($second, ref($first)) attr-dict";
   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.
 
 def PropertiesWithCustomPrint : Property<"PropertiesWithCustomPrint"> {
   let convertToAttribute = [{
     getPropertiesAsAttribute($_ctxt, $_storage)
   }];
   let convertFromAttribute = [{

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

)",
   writePropertiesMethod << "  auto &prop = getProperties(); (void)prop;\n";
   for (const auto &attrOrProp : attrOrProperties) {
     if (const auto *namedProperty =
             attrOrProp.dyn_cast<const NamedProperty *>()) {
       StringRef name = namedProperty->name;
       FmtContext fctx;
       fctx.addSubst("_reader", "reader")
           .addSubst("_writer", "writer")
-          .addSubst("_storage", propertyStorage);
+          .addSubst("_storage", propertyStorage)
+          .addSubst("_ctxt", "this->getContext()");
       readPropertiesMethod << formatv(
           R"(
   {{
     auto &propStorage = prop.{0};
     auto readProp = [&]() {
       {1};
       return ::mlir::success();
     };