[mlir][complex] Custom attribute comlex.number.

[Lewuathe]

Summary

Add custom attribute for complex dialect. Although this commit does not have significant impact on the conversion framework, it will lead us to construct complex numbers in a readable and tidy manner.

Related discussion: https://reviews.llvm.org/D127476

[pifon2a]

Thank you!

[pifon2a]

mlir/include/mlir/Dialect/Complex/IR/ComplexAttributes.td

38

shouldn't it be

AttributeSelfTypeParameter<"">:$type,
APFloatParameter<"">:$lhs,
APFloatParameter<"">:$rhs

and then we probably need some verification similar to BuiltinAttributes.cpp:293 that checks that both the types of $lhs, $rhs and $type match?

40

In that case the assemblyFormat should also have $type.

let assemblyFormat = "`<` $real `,` $imag `>` `:` $type

[Lewuathe]

mlir/include/mlir/Dialect/Complex/IR/ComplexAttributes.td

38

Looks like simply using APFloatParameter does not work. Do we need to provide the custom field parser for APFloatParameter?

/Users/sasakikai/dev/llvm-project/build/tools/mlir/include/mlir/Dialect/Complex/IR/ComplexAttributes.cpp.inc:84:26: error: implicit instantiation of undefined template 'mlir::FieldParser<llvm::APFloat>'
  _result_real = ::mlir::FieldParser<::llvm::APFloat>::parse(odsParser);
                         ^
/Users/sasakikai/dev/llvm-project/mlir/include/mlir/IR/DialectImplementation.h:58:8: note: template is declared here
struct FieldParser;
       ^
In file included from /Users/sasakikai/dev/llvm-project/mlir/lib/Dialect/Complex/IR/ComplexDialect.cpp:45:
/Users/sasakikai/dev/llvm-project/build/tools/mlir/include/mlir/Dialect/Complex/IR/ComplexAttributes.cpp.inc:93:26: error: implicit instantiation of undefined template 'mlir::FieldParser<llvm::APFloat>'
  _result_imag = ::mlir::FieldParser<::llvm::APFloat>::parse(odsParser);
                         ^
/Users/sasakikai/dev/llvm-project/mlir/include/mlir/IR/DialectImplementation.h:58:8: note: template is declared here
struct FieldParser;
       ^
2 errors generated.

40

Using type in the assembly format may not be allowed?

error: unexpected self type parameter in assembly format
`<` $real `,` $imag `>` `:` $type

[Lewuathe]

Add verifier checking the given types.

[pifon2a]

mlir/include/mlir/Dialect/Complex/IR/ComplexAttributes.td

38

Yes, in that case we need to have a custom print/parser. Take a look at CombiningKindAttr in VectorOps.h. There is

void print(AsmPrinter &p) const;
static Attribute parse(AsmParser &parser, Type type);

defined for it.

If FloatAttr is used, then we store the type twice for no good reason.

[Lewuathe]

@pifon2a Thank you so much. Let me have a look.

[Lewuathe]

Update to use APFloatParameter for real and imaginary parts.

[pifon2a]

mlir/lib/Dialect/Complex/IR/ComplexDialect.cpp

54

Float semantics of imag and real should be verified, I think:

  // Verify that the type semantics match that of the `real` and `imag` values.
auto typeFloatSemantics = type.cast<FloatType>().getFloatSemantics() ;
if (&typeFloatSemantics != &real.getSemantics()) 
  return emitError()  << "type doesn't match the type implied by its `real` value";
if (&typeFloatSemantics != &imag.getSemantics()) 
  return emitError()  << "type doesn't match the type implied by its `imag` value";

64

interesting. The type is not printed here, but it is present in the test output.

[pifon2a]

Thank you @Lewuathe ! This looks good.

[akuegel]

I think it may also help to look at DenseArrayAttr as an example. It uses this syntax:
[:f64 0, 2]

So based on this, we could use the custom format
#complex.number<:f64 0, 2>

which would have the nice effect that we know what element type to parse before we parse the elements.
Also, I think it would be preferable if the type of the attribute is of ComplexType, so for example if element type is F64, then ComplexType::get(FloatType::getF64(context))

[pifon2a]

Good point, Adrian.

In D130149#3675644, @akuegel wrote:

I think it may also help to look at DenseArrayAttr as an example. It uses this syntax:
[:f64 0, 2]

So based on this, we could use the custom format
#complex.number<:f64 0, 2>

which would have the nice effect that we know what element type to parse before we parse the elements.
Also, I think it would be preferable if the type of the attribute is of ComplexType, so for example if element type is F64, then ComplexType::get(FloatType::getF64(context))

[Lewuathe]

Refine format to be #complex.number<f64 1.0, 2.0>

[Lewuathe]

Thank you both. I updated the change accordingly. But the following case gets invalid with the semantic validation. f32 is not compatible with the given floating point numbers?

func.func @number_attr_f32() {
  "test.number_attr"() {
    // expected-error @+1 {{unexpected error: type doesn't match the type implied by its `real` value}}
    attr = #complex.number<:f32 1.0, 0.0>
  } : () -> ()

  return
}

[akuegel]

mlir/include/mlir/Dialect/Complex/IR/ComplexAttributes.td

37

I wonder whether it would make sense to use FloatAttr here?

Something like:

let parameters = (ins AttributeSelfTypeParameter<"">:$type, "mlir::FloatAttr":$real, "mlir::FloatAttr":$imag);

And then you could have a convenience builder that also allows to build from APFloat:

let builders = [
  AttrBuilderWithInferredContext<
      (ins "ComplexType":$type,
           "APFloat":$real, "APFloat":$imag), [{
    auto elementType = type.getElementType();
    auto realFloat = FloatAttr::get(elementType, real);
    auto imagFloat = FloatAttr::get(elementType, imag);
    return $_get(type.getContext(), type, realFloat, imagFloat);
  }]>
];

mlir/lib/Dialect/Complex/IR/ComplexDialect.cpp

100

I believe you would have to convert the APFloats to the required float semantics. The method signature for this is:

convert(const fltSemantics &ToSemantics, roundingMode RM, bool *losesInfo);

APFloat realFloat(real);
bool unused;
realFloat.convert(type.cast<FloatType>().getFloatSemantics(), APFloat::rmNearestTiesToEven, &unused);

But I think this would be taken care of automatically if you use FloatAttr as storage (see other comment).

[pifon2a]

I wonder whether it would make sense to use FloatAttr here?

@akuegel FloatAttr has two fields

AttributeSelfTypeParameter<"">:$type,
APFloatParameter<"">:$value

I am not sure we want $type to be stored three times.

[akuegel]

In D130149#3678844, @pifon2a wrote:

@akuegel FloatAttr has two fields

AttributeSelfTypeParameter<"">:$type,
APFloatParameter<"">:$value

I am not sure we want $type to be stored three times.

Ok, good point. So I guess then the convert calls need to be done in the complex::NumberAttr::parse like I suggested, so that the correct APFloat is passed.

[Lewuathe]

Support f32 by converting the APFloat.

[Lewuathe]

@pifon2a @akuegel Thank you for the feedback! I updated the patch accordingly, and it's now supporting f32 type too.

[akuegel]

Thank you :)

[Mogball]

Hey, I just saw this. There are quite a few instances of storing complex numbers as an array attribute of two elements [0.0 : f32, 0.0 : f32]. Now that there is a better way of doing that, do you plan on refactoring some of the dialects to use this?

If not, can you file an issue to track that? Thanks!

[akuegel]

In D130149#3689857, @Mogball wrote:

Hey, I just saw this. There are quite a few instances of storing complex numbers as an array attribute of two elements [0.0 : f32, 0.0 : f32]. Now that there is a better way of doing that, do you plan on refactoring some of the dialects to use this?

If not, can you file an issue to track that? Thanks!

Yes, the plan is to replace ArrayAttr with this new attribute. Lewuathe is already working on it:

https://discourse.llvm.org/t/multiple-type-support-for-input-attribute/64116

mlir/include/mlir/Dialect/Complex/IR/CMakeLists.txt

 add_mlir_dialect(ComplexOps complex)
 add_mlir_doc(ComplexOps ComplexOps Dialects/ -gen-dialect-doc)
+
+set(LLVM_TARGET_DEFINITIONS ComplexAttributes.td)
+mlir_tablegen(ComplexAttributes.h.inc -gen-attrdef-decls)
+mlir_tablegen(ComplexAttributes.cpp.inc -gen-attrdef-defs)
+add_public_tablegen_target(MLIRComplexAttributesIncGen)

mlir/include/mlir/Dialect/Complex/IR/Complex.h

 
 //===----------------------------------------------------------------------===//
 // Complex Dialect Operations
 //===----------------------------------------------------------------------===//
 
 #define GET_OP_CLASSES
 #include "mlir/Dialect/Complex/IR/ComplexOps.h.inc"
 
+#define GET_ATTRDEF_CLASSES
+#include "mlir/Dialect/Complex/IR/ComplexAttributes.h.inc"
+
 #endif // MLIR_DIALECT_COMPLEX_IR_COMPLEX_H_

mlir/include/mlir/Dialect/Complex/IR/ComplexAttributes.td

+//===- ComplexAttributes.td - Definitions for complex attributes -*- tablegen -*-=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef COMPLEX_ATTRIBUTE
+#define COMPLEX_ATTRIBUTE
+
+include "mlir/IR/AttrTypeBase.td"
+include "mlir/Dialect/Complex/IR/ComplexBase.td"
+
+//===----------------------------------------------------------------------===//
+// Complex Attributes.
+//===----------------------------------------------------------------------===//
+
+class Complex_Attr<string attrName, string attrMnemonic>
+    : AttrDef<Complex_Dialect, attrName> {
+  let mnemonic = attrMnemonic;
+}
+
+def Complex_NumberAttr : Complex_Attr<"Number", "number"> {
+  let summary = "A complex number attribute";
+
+  let description = [{
+    A complex number attribute.
+
+    Example:
+
+    ```mlir
+    #complex.number<:f64 1.0, 2.0>
+    ```
+  }];
+
+  let parameters = (ins APFloatParameter<"">:$real, APFloatParameter<"">:$imag, AttributeSelfTypeParameter<"">:$type);
+
+  let genVerifyDecl = 1;
+  let hasCustomAssemblyFormat = 1;
+}
+
+#endif // COMPLEX_ATTRIBUTE

mlir/include/mlir/Dialect/Complex/IR/ComplexBase.td

def Complex_Dialect : Dialect {
   let description = [{
     The complex dialect is intended to hold complex numbers creation and
     arithmetic ops.
   }];
 
   let dependentDialects = ["arith::ArithmeticDialect"];
   let hasConstantMaterializer = 1;
   let emitAccessorPrefix = kEmitAccessorPrefix_Prefixed;
+  let useDefaultAttributePrinterParser = 1;
 }
 
 #endif // COMPLEX_BASE

mlir/lib/Dialect/Complex/IR/CMakeLists.txt

 add_mlir_dialect_library(MLIRComplexDialect
   ComplexOps.cpp
   ComplexDialect.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Dialect/Complex
 
   DEPENDS
   MLIRComplexOpsIncGen
+  MLIRComplexAttributesIncGen
 
   LINK_LIBS PUBLIC
   MLIRArithmeticDialect
   MLIRDialect
   MLIRInferTypeOpInterface
   MLIRIR
   )

mlir/lib/Dialect/Complex/IR/ComplexDialect.cpp

 //===- ComplexDialect.cpp - MLIR Complex Dialect --------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Dialect/Arithmetic/IR/Arithmetic.h"
 #include "mlir/Dialect/Complex/IR/Complex.h"
+#include "mlir/IR/Builders.h"
+#include "mlir/IR/DialectImplementation.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/TypeSwitch.h"
 
 using namespace mlir;
 
 #include "mlir/Dialect/Complex/IR/ComplexOpsDialect.cpp.inc"
 
 void complex::ComplexDialect::initialize() {
   addOperations<
 #define GET_OP_LIST
 #include "mlir/Dialect/Complex/IR/ComplexOps.cpp.inc"
       >();
+  addAttributes<
+#define GET_ATTRDEF_LIST
+#include "mlir/Dialect/Complex/IR/ComplexAttributes.cpp.inc"
+      >();
 }
 
 Operation *complex::ComplexDialect::materializeConstant(OpBuilder &builder,
                                                         Attribute value,
                                                         Type type,
                                                         Location loc) {
   if (complex::ConstantOp::isBuildableWith(value, type)) {
     return builder.create<complex::ConstantOp>(loc, type,
                                                value.cast<ArrayAttr>());
   }
   if (arith::ConstantOp::isBuildableWith(value, type))
     return builder.create<arith::ConstantOp>(loc, type, value);
   return nullptr;
 }
+
+#define GET_ATTRDEF_CLASSES
+#include "mlir/Dialect/Complex/IR/ComplexAttributes.cpp.inc"
+
+LogicalResult complex::NumberAttr::verify(
+    ::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError,
+    ::llvm::APFloat real, ::llvm::APFloat imag, ::mlir::Type type) {
+
+  if (!type.isa<FloatType>())
+    return emitError()
+           << "element of the complex attribute must be float like type.";
+
+  const auto &typeFloatSemantics = type.cast<FloatType>().getFloatSemantics();
+  if (&real.getSemantics() != &typeFloatSemantics)
+    return emitError()
+           << "type doesn't match the type implied by its `real` value";
+  if (&imag.getSemantics() != &typeFloatSemantics)
+    return emitError()
+           << "type doesn't match the type implied by its `imag` value";
+
+  return success();
+}
+
+void complex::NumberAttr::print(AsmPrinter &printer) const {
+  printer << "<:";
+  printer.printType(getType());
+  printer << " ";
+  printer.printFloat(getReal());
+  printer << ", ";
+  printer.printFloat(getImag());
+  printer << ">";
+}
+
+Attribute complex::NumberAttr::parse(AsmParser &parser, Type odsType) {
+  if (failed(parser.parseLess()))
+    return {};
+
+  if (failed(parser.parseColon()))
+    return {};
+
+  Type type;
+  if (failed(parser.parseType(type)))
+    return {};
+
+  double real;
+  if (failed(parser.parseFloat(real)))
+    return {};
+
+  if (failed(parser.parseComma()))
+    return {};
+
+  double imag;
+  if (failed(parser.parseFloat(imag)))
+    return {};
+
+  if (failed(parser.parseGreater()))
+    return {};
+
+  return NumberAttr::get(parser.getContext(), APFloat(real), APFloat(imag),
+                         type);
+}

mlir/test/Dialect/Complex/attribute.mlir

+// RUN: mlir-opt %s -allow-unregistered-dialect -verify-diagnostics | FileCheck %s
+
+func.func @number_attr_f64() {
+  "test.number_attr"() {
+    // CHECK: attr = #complex.number<:f64 1.000000e+00, 0.000000e+00> : f64
+    attr = #complex.number<:f64 1.0, 0.0>
+  } : () -> ()
+
+  return
+}