[Mlir-commits] [mlir] 82bbebb - [mlir][python] Replace PythonAttr mappings with downcasting

[Rahul Kayaith]

Author: Rahul Kayaith
Date: 2023-07-10T22:01:35-04:00
New Revision: 82bbebbfc257255abf4d861c2ec7542220e42993

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

LOG: [mlir][python] Replace PythonAttr mappings with downcasting

Since op `Attribute`s are automatically downcasted on access, these mappings
aren't necessary anymore. Instead we just always generate the getters/setters
for attributes even if there isn't a `PythonAttr` mapping.

depends on D154462

Reviewed By: ftynse

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

Added: 
    

Modified: 
    mlir/include/mlir/Bindings/Python/Attributes.td
    mlir/test/mlir-tblgen/op-python-bindings.td
    mlir/tools/mlir-tblgen/OpPythonBindingGen.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Bindings/Python/Attributes.td b/mlir/include/mlir/Bindings/Python/Attributes.td
index f9a7fa703f5017..c5947c62ddf460 100644
--- a/mlir/include/mlir/Bindings/Python/Attributes.td
+++ b/mlir/include/mlir/Bindings/Python/Attributes.td
@@ -21,14 +21,4 @@ class PythonAttr<string c, string p> {
   string pythonType = p;
 }
 
-// Mappings between supported builtin attribtues and Python types.
-def : PythonAttr<"::mlir::Attribute", "_ods_ir.Attribute">;
-def : PythonAttr<"::mlir::BoolAttr", "_ods_ir.BoolAttr">;
-def : PythonAttr<"::mlir::IntegerAttr", "_ods_ir.IntegerAttr">;
-def : PythonAttr<"::mlir::FloatAttr", "_ods_ir.FloatAttr">;
-def : PythonAttr<"::mlir::StringAttr", "_ods_ir.StringAttr">;
-def : PythonAttr<"::mlir::DenseElementsAttr", "_ods_ir.DenseElementsAttr">;
-def : PythonAttr<"::mlir::DenseIntElementsAttr", "_ods_ir.DenseIntElementsAttr">;
-def : PythonAttr<"::mlir::DenseFPElementsAttr", "_ods_ir.DenseFPElementsAttr">;
-
 #endif

diff  --git a/mlir/test/mlir-tblgen/op-python-bindings.td b/mlir/test/mlir-tblgen/op-python-bindings.td
index 97fe306d6c1601..7513edae418551 100644
--- a/mlir/test/mlir-tblgen/op-python-bindings.td
+++ b/mlir/test/mlir-tblgen/op-python-bindings.td
@@ -1,6 +1,7 @@
 // RUN: mlir-tblgen -gen-python-op-bindings -bind-dialect=test -I %S/../../include %s | FileCheck %s
 
 include "mlir/IR/OpBase.td"
+include "mlir/IR/AttrTypeBase.td"
 include "mlir/Interfaces/InferTypeOpInterface.td"
 include "mlir/Bindings/Python/Attributes.td"
 
@@ -12,6 +13,9 @@ def Test_Dialect : Dialect {
   let name = "test";
   let cppNamespace = "Test";
 }
+def TestAttr : AttrDef<Test_Dialect, "TestAttr">;
+def : PythonAttr<TestAttr.cppType, "<TestAttr hook>">;
+
 class TestOp<string mnemonic, list<Trait> traits = []> :
     Op<Test_Dialect, mnemonic, traits>;
 
@@ -110,7 +114,7 @@ def AttrSizedResultsOp : TestOp<"attr_sized_results",
 // CHECK-NOT: _ODS_OPERAND_SEGMENTS
 // CHECK-NOT: _ODS_RESULT_SEGMENTS
 def AttributedOp : TestOp<"attributed_op"> {
-  // CHECK: def __init__(self, i32attr, in_, *, optionalF32Attr=None, unitAttr=None, loc=None, ip=None):
+  // CHECK: def __init__(self, i32attr, in_, test_attr, *, optionalF32Attr=None, unitAttr=None, loc=None, ip=None):
   // CHECK:   operands = []
   // CHECK:   results = []
   // CHECK:   attributes = {}
@@ -130,13 +134,13 @@ def AttributedOp : TestOp<"attributed_op"> {
 
   // CHECK: @builtins.property
   // CHECK: def i32attr(self):
-  // CHECK:   return _ods_ir.IntegerAttr(self.operation.attributes["i32attr"])
+  // CHECK:   return (self.operation.attributes["i32attr"])
 
   // CHECK: @builtins.property
   // CHECK: def optionalF32Attr(self):
   // CHECK:   if "optionalF32Attr" not in self.operation.attributes:
   // CHECK:     return None
-  // CHECK:   return _ods_ir.FloatAttr(self.operation.attributes["optionalF32Attr"])
+  // CHECK:   return (self.operation.attributes["optionalF32Attr"])
 
   // CHECK: @builtins.property
   // CHECK: def unitAttr(self):
@@ -144,9 +148,13 @@ def AttributedOp : TestOp<"attributed_op"> {
 
   // CHECK: @builtins.property
   // CHECK: def in_(self):
-  // CHECK:   return _ods_ir.IntegerAttr(self.operation.attributes["in"])
+  // CHECK:   return (self.operation.attributes["in"])
+
+  // CHECK: @builtins.property
+  // CHECK: def test_attr(self):
+  // CHECK:   return <TestAttr hook>(self.operation.attributes["test_attr"])
   let arguments = (ins I32Attr:$i32attr, OptionalAttr<F32Attr>:$optionalF32Attr,
-                   UnitAttr:$unitAttr, I32Attr:$in);
+                   UnitAttr:$unitAttr, I32Attr:$in, TestAttr:$test_attr);
 }
 
 // CHECK: @_ods_cext.register_operation(_Dialect)
@@ -178,7 +186,7 @@ def AttributedOpWithOperands : TestOp<"attributed_op_with_operands"> {
   // CHECK: def is_(self):
   // CHECK:   if "is" not in self.operation.attributes:
   // CHECK:     return None
-  // CHECK:   return _ods_ir.FloatAttr(self.operation.attributes["is"])
+  // CHECK:   return (self.operation.attributes["is"])
   let arguments = (ins I32, UnitAttr:$in, F32, OptionalAttr<F32Attr>:$is);
 }
 

diff  --git a/mlir/tools/mlir-tblgen/OpPythonBindingGen.cpp b/mlir/tools/mlir-tblgen/OpPythonBindingGen.cpp
index 6e4a9e347cd6dc..d25c4b30e8d0b9 100644
--- a/mlir/tools/mlir-tblgen/OpPythonBindingGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpPythonBindingGen.cpp
@@ -468,10 +468,6 @@ static void emitAttributeAccessors(const Operator &op,
       continue;
     }
 
-    // Other kinds of attributes need a mapping to a Python type.
-    if (!attributeClasses.count(namedAttr.attr.getStorageType().trim()))
-      continue;
-
     StringRef pythonType =
         attributeClasses.lookup(namedAttr.attr.getStorageType());
     if (namedAttr.attr.isOptional()) {