[Calyx] constant op

[jiahanxie353]

This patch tries to support Calyx constant op whose type is always unsigned integer; and the values and user-intended types are only specified via attributes

[jiahanxie353]

Hi @rachitnigam @andrewb1999 @cgyurgyik , I'm trying to support the feature we discussed in here.

When I'm running example:

module {
  func.func @main() -> i32 {
    %0 = calyx.constant {value=4.2 : f32} : i32
    return %0 : i32
  }
}

it tells me:

calyx-const.mlir:4:42: error: expected attribute value
    %0 = calyx.constant {value=4.2 : f32} : i32
                                                                    ^

so then I changed to:

%0 = "calyx.constant"() <{value=4.2 : f32}> : () -> i32

it again complains:

calyx-const.mlir:4:10: error: 'calyx.constant' op requires string attribute 'sym_name'
    %0 = "calyx.constant"() <{value=4.2 : f32}> : () -> i32
             ^
calyx-const.mlir:4:10: note: see current operation: %0 = "calyx.constant"() <{value = 4.200000e+00 : f32}> : () -> i3

and then I changed to:

%0 = "calyx.constant"() {sym_name="cst_0", value=4.2 : f32} : () -> i32

it complains:

calyx-const.mlir:4:10: error: 'calyx.constant' op symbol's parent must have the SymbolTable trait
    %0 = "calyx.constant"() {sym_name="cst_0", value=4.2 : f32} : () -> i32
              ^
calyx-const.mlir:4:10: note: see current operation: %0 = "calyx.constant"() <{value = 4.200000e+00 : f32}> {sym_name = "cst_0"} : () -> i32

Any idea how to fix it?

[andrewb1999]

I think your assembly format is wrong. I also think you shouldn't use { } for this since that's usually reserved for regions and generic op printing. Maybe an assembly format like this?

let assemblyFormat = "$sym_name ` ` `<` $value `>` `:` qualified(type($out))";

Also we need to remove the FirstAttrDerivedResultType property from the operation because the output type no longer necessarily matches the attribute type.

[jiahanxie353]

I think your assembly format is wrong. I also think you shouldn't use { } for this since that's usually reserved for regions and generic op printing. Maybe an assembly format like this?

let assemblyFormat = "$sym_name ` ` `<` $value `>` `:` qualified(type($out))";

Also we need to remove the FirstAttrDerivedResultType property from the operation because the output type no longer necessarily matches the attribute type.

I see, thanks!

Is attr-dict a required field in assemblyFormat?

error: 'attr-dict' directive not found in custom assembly format
$sym_name ` ` `<` $value `>` `:` qualified(type($out))
^
Included from /scratch/jiahan/circt/include/circt/Dialect/Calyx/Calyx.td:95:

My td file has:

  let arguments = (ins SymbolNameAttr:$sym_name, TypedAttrInterface:$value);

  let results = (outs AnySignlessInteger:$out);

  let builders = [
    OpBuilder <(ins "StringRef":$sym_name, "TypedAttr":$attr)>,
  ];

  let hasFolder = 1;
  let assemblyFormat = "$sym_name ` ` `<` $value `>` `:` qualified(type($out))";
  let hasVerifier = 1;

any obvious mistake?

[andrewb1999]

Yeah my bad I forgot to add that to the format. Go ahead and add it right before the ':'

[jiahanxie353]

Yeah my bad I forgot to add that to the format. Go ahead and add it right before the ':'

No worries and thanks!

Now I have:

def ConstantOp: CalyxPrimitive<"constant",
    [ConstantLike, DeclareOpInterfaceMethods<OpAsmOpInterface, ["getAsmResultNames"]>
    ]> {
  let arguments = (ins SymbolNameAttr:$sym_name, TypedAttrInterface:$value);

  let results = (outs AnySignlessInteger:$out);

  let builders = [
    OpBuilder <(ins "StringRef":$sym_name, "TypedAttr":$attr)>,
  ];

  let hasFolder = 1;
  let assemblyFormat = "$sym_name ` ` `<` $value `>` attr-dict `:` qualified(type($out))";
  let hasVerifier = 1;
}

but it complains:

calyx-const.mlir:3:10: error: 'calyx.constant' op symbol's parent must have the SymbolTable trait
    %0 = calyx.constant @cst <4.2 : f32> : i32
              ^
calyx-const.mlir:3:10: note: see current operation: %0 = "calyx.constant"() <{sym_name = "cst", value = 4.200000e+00 : f32}> : () -> i32

I mean my example is:

module {
  func.func @main() -> i32 {
    %0 = calyx.constant @cst <4.2 : f32> : i32
    return %0 : i32
  }
}

and calyx.constant's parent is funcOp, which obviously has the SymbolTable trait. This is very confusing (and TableGen is confusing for me in general...

[andrewb1999]

Can you push what you have currently so I can take a closer look?

[jiahanxie353]

Can you push what you have currently so I can take a closer look?

Sure, just pushed, thank you!

[andrewb1999]

As can be seen here the func op does not have the SymbolTable trait (in contrast to the Calyx ComponentOp as can be seen here. This is in contrast to the Symbol trait which just says an operation defines a symbol. The SymbolTable trait defines the ability for an op to have children that define a symbol. It seems FuncOp does not have this trait to prevent people from defining functions inside of another function.

I think you can simply switch to testing with a calyx.component instead of func.func.

[jiahanxie353]

As can be seen here the func op does not have the SymbolTable trait

Indeed, I always assumed that it does...

The SymbolTable trait defines the ability for an op to have children that define a symbol. It seems FuncOp does not have this trait to prevent people from defining functions inside of another function.

I see, makes sense. Thanks!

I think you can simply switch to testing with a calyx.component instead of func.func.

Sounds good!

[cgyurgyik]

Seems good to me. Let's let @andrewb1999 take a second look.

[andrewb1999]

LGTM! Go ahead and merge once CI passes.