[Mlir-commits] [mlir] 89af17c - Define a `cppAccessorType` to const-ref in APFloatParameter and update ODS emitter to use it for verifier signatures

[Mehdi Amini]

Author: Mehdi Amini
Date: 2022-01-03T04:57:11Z
New Revision: 89af17c0c74eb9d8d11870f6510e475eff74eef4

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

LOG: Define a `cppAccessorType` to const-ref in APFloatParameter and update ODS emitter to use it for verifier signatures

This reduce an unnecessary amount of copy of non-trivial objects, like
APFloat.

Reviewed By: rriddle, jpienaar

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

Added: 
    

Modified: 
    mlir/include/mlir/IR/OpBase.td
    mlir/lib/IR/BuiltinAttributes.cpp
    mlir/test/lib/Dialect/Test/TestAttributes.cpp
    mlir/test/mlir-tblgen/attrdefs.td
    mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td
index f1a5446ad1f97..e46b8fb35ec7c 100644
--- a/mlir/include/mlir/IR/OpBase.td
+++ b/mlir/include/mlir/IR/OpBase.td
@@ -3135,6 +3135,7 @@ class StringRefParameter<string desc = ""> :
 class APFloatParameter<string desc> :
     AttrOrTypeParameter<"::llvm::APFloat", desc> {
   let comparator = "$_lhs.bitwiseIsEqual($_rhs)";
+  let cppAccessorType = "const ::llvm::APFloat &";
 }
 
 // For standard ArrayRefs, which require allocation.

diff  --git a/mlir/lib/IR/BuiltinAttributes.cpp b/mlir/lib/IR/BuiltinAttributes.cpp
index 802df2dada9d0..2a34ed34e7d19 100644
--- a/mlir/lib/IR/BuiltinAttributes.cpp
+++ b/mlir/lib/IR/BuiltinAttributes.cpp
@@ -283,7 +283,7 @@ double FloatAttr::getValueAsDouble(APFloat value) {
 }
 
 LogicalResult FloatAttr::verify(function_ref<InFlightDiagnostic()> emitError,
-                                Type type, APFloat value) {
+                                Type type, const APFloat &value) {
   // Verify that the type is correct.
   if (!type.isa<FloatType>())
     return emitError() << "expected floating point type";

diff  --git a/mlir/test/lib/Dialect/Test/TestAttributes.cpp b/mlir/test/lib/Dialect/Test/TestAttributes.cpp
index 3a860994f0e84..909f5b3995439 100644
--- a/mlir/test/lib/Dialect/Test/TestAttributes.cpp
+++ b/mlir/test/lib/Dialect/Test/TestAttributes.cpp
@@ -129,7 +129,7 @@ TestI64ElementsAttr::verify(function_ref<InFlightDiagnostic()> emitError,
 
 LogicalResult
 TestAttrWithFormatAttr::verify(function_ref<InFlightDiagnostic()> emitError,
-                               int64_t one, std::string two, IntegerAttr three,
+                               int64_t one, StringRef two, IntegerAttr three,
                                ArrayRef<int> four) {
   if (four.size() != static_cast<unsigned>(one))
     return emitError() << "expected 'one' to equal 'four.size()'";

diff  --git a/mlir/test/mlir-tblgen/attrdefs.td b/mlir/test/mlir-tblgen/attrdefs.td
index 34c8588225f70..f53705dd662fc 100644
--- a/mlir/test/mlir-tblgen/attrdefs.td
+++ b/mlir/test/mlir-tblgen/attrdefs.td
@@ -61,8 +61,8 @@ def B_CompoundAttrA : TestAttr<"CompoundA"> {
   let genVerifyDecl = 1;
 
 // DECL-LABEL: class CompoundAAttr : public ::mlir::Attribute
-// DECL: static CompoundAAttr getChecked(::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError, ::mlir::MLIRContext *context, int widthOfSomething, ::test::SimpleTypeA exampleTdType, ::llvm::APFloat apFloat, ::llvm::ArrayRef<int> dims, ::mlir::Type inner);
-// DECL: static ::mlir::LogicalResult verify(::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError, int widthOfSomething, ::test::SimpleTypeA exampleTdType, ::llvm::APFloat apFloat, ::llvm::ArrayRef<int> dims, ::mlir::Type inner);
+// DECL: static CompoundAAttr getChecked(::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError, ::mlir::MLIRContext *context, int widthOfSomething, ::test::SimpleTypeA exampleTdType, const ::llvm::APFloat &apFloat, ::llvm::ArrayRef<int> dims, ::mlir::Type inner);
+// DECL: static ::mlir::LogicalResult verify(::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError, int widthOfSomething, ::test::SimpleTypeA exampleTdType, const ::llvm::APFloat &apFloat, ::llvm::ArrayRef<int> dims, ::mlir::Type inner);
 // DECL: static constexpr ::llvm::StringLiteral getMnemonic() {
 // DECL:   return {"cmpnd_a"};
 // DECL: }
@@ -71,7 +71,7 @@ def B_CompoundAttrA : TestAttr<"CompoundA"> {
 // DECL: void print(::mlir::AsmPrinter &printer) const;
 // DECL: int getWidthOfSomething() const;
 // DECL: ::test::SimpleTypeA getExampleTdType() const;
-// DECL: ::llvm::APFloat getApFloat() const;
+// DECL: const ::llvm::APFloat &getApFloat() const;
 
 // Check that AttributeSelfTypeParameter is handled properly.
 // DEF-LABEL: struct CompoundAAttrStorage
@@ -139,5 +139,5 @@ def F_ParamWithAccessorTypeAttr : TestAttr<"ParamWithAccessorType"> {
 // DECL-LABEL: class ParamWithAccessorTypeAttr
 // DECL: StringRef getParam()
 // DEF: ParamWithAccessorTypeAttrStorage
-// DEF: ParamWithAccessorTypeAttrStorage(std::string param)
+// DEF: ParamWithAccessorTypeAttrStorage(StringRef param)
 // DEF: StringRef ParamWithAccessorTypeAttr::getParam()

diff  --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
index d90adbc47ef4d..f2df6c8ae7651 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -315,7 +315,7 @@ DefGen::getBuilderParams(std::initializer_list<MethodParameter> prefix) const {
   SmallVector<MethodParameter> builderParams;
   builderParams.append(prefix.begin(), prefix.end());
   for (auto &param : params)
-    builderParams.emplace_back(param.getCppType(), param.getName());
+    builderParams.emplace_back(param.getCppAccessorType(), param.getName());
   return builderParams;
 }