Changing the default argument for child classes

So I have the following:

class Parent<string mnemonic, list<OpTrait> traits = []> {
    let assemblyFormat = "$instanceName attr-dict `:` type(results)";
    let arguments = (ins StrAttr:$instanceName, StrArrayAttr:$portNames);
    let skipDefaultBuilders = 1;
}

def OpOne : Parent<"op1", []> { ... }
def OpTwo : Parent<"op2", []> { ... }

Each op that inherits from Parent must have a set of port names. However, these port names are different, and known statically (e.g. each instance of Op1 will always have left, right, and out ports).

Initially I defined arguments separately for each operation with a default argument for portNames.

  1. Is it possible to provide default arguments for the ops, even though let arguments = ... is defined in the base class Parent

  2. Is there a better way to do this? I need to provide these port names in 3 different spots:

// Definition for OpOne
let results = (outs AnyType:$left, AnyType:$right, AnyType:$out);
let arguments = (ins ..., DefaultValuedAttr<StrArrayAttr, "{\"left\", \"right\", \"out\", ">:$portNames)
let builders = [
  ...
  $state.addAttribute("portNames", StrArrayAttr({"left", "right", "out"}));
]

It would be awesome to define the port names once for each op, and do most of the heavy lifting in the Parent base class.

You mean beyond specifying let arguments again in derived class?

So you could use a templated base class with the names as template args and then use tablegen macros to compose them. You can also sort of make tablegen functions (define a templated class and then query a member of interest that constructed the expression of interest) to help be building blocks rather than one class - although if you only want.to specify it once, then you may have to rely on tablegen and creating the dag of the arguments “programatically” and having finally a templated base class.

Now portNames I would probably not have as an argument here. It doesn’t seem to define the op or it’s behavior. It seems more like it is a static, fixed-per-op-kind mapping from output index to a name being stored as an attribute (now you could even take advantage of derived attribute here & C++ helper function to instead provide an accessor per op that returns a vector of strings, then probably won’t need the custom builder either; or (probably simpler) just make it a member function by way of extra C++ defs and return constant map or some such). Or are there cases where the names in the attribute will be changed from the default and the names used for the port output names?

You mean beyond specifying let arguments again in derived class?

Yeah this would seemingly defeat the purpose.

Now portNames I would probably not have as an argument here. It doesn’t seem to define the op or it’s behavior. It seems more like it is a static, fixed-per-op-kind mapping from output index

Precisely. The names will never be changed. I initially did this instead of having an attribute:

  let extraClassDeclaration = [{
    SmallVector<StringRef> portNames() { return {"left", "right", "out"}; }
  }];

While it works, it just doesn’t seem right, nor can I verify that the function portNames is defined in a similar way that operations can be checked for attributes.
(Edit: It seems this may be along the lines of a derived attribute? Is there code examples / documentation for this?)

It seems you’re suggesting to still use an IntAttr that refers to some index in a map, as seen here:

fixed-per-op-kind mapping from output index to a name being stored as an attribute

Can you elaborate? Where exactly would a mapping like this be stored? Also, how is this any different than storing the port names directly?

You’re probably looking for Op Interfaces: llvm-project/Interfaces.md at main · llvm/llvm-project (github.com)

Since it seems like you want per-op-type attributes: Parameterizable Op Traits? - MLIR - LLVM Discussion Forums I never got around to investigating further.

1 Like

Hmm this seems close to what I want. Thank you. Unfortunately, I am getting some errors that aren’t really making sense to me.

I have:

def CellInterface : OpInterface<"CellInterface"> {
  let cppNamespace = "::circt::calyx";
  let description = [{ ... }];
  let methods = [
    InterfaceMethod<
      "...",
      "SmallVector<StringRef>", "portNames"
    >
  ];
}

class CalyxCell<string mnemonic, list<OpTrait> traits = []> :
  CalyxOp<mnemonic, !listconcat(traits, [
    DeclareOpInterfaceMethods<CellInterface>
  ])> {
}

which leads to the following error(s):

include/circt/Dialect/Calyx/Calyx.h.inc:588:192: error: no member named 'CellInterface' in namespace 'circt::calyx'
class InstanceOp : public ::mlir::Op<InstanceOp, ::mlir::OpTrait::ZeroRegion, ::mlir::OpTrait::VariadicResults, ::mlir::OpTrait::ZeroSuccessor, ::mlir::OpTrait::ZeroOperands, ::circt::calyx::CellInterface::Trait> {
                                                                                                                                                                               ~~~~~~~~~~~~~~~~^
include/circt/Dialect/Calyx/Calyx.h.inc:590:9: error: use of undeclared identifier 'Op'
  using Op::Op;
        ^
include/circt/Dialect/Calyx/Calyx.h.inc:591:9: error: use of undeclared identifier 'Op'
  using Op::print;
        ^
include/circt/Dialect/Calyx/Calyx.h.inc:638:44: error: member reference type 'circt::calyx::InstanceOp' is not a pointer; did you mean to use '.'?
    return getAttributeNameForIndex((*this)->getName(), index);
                                    ~~~~~~~^~
                                           .
include/circt/Dialect/Calyx/Calyx.h.inc:638:46: error: no member named 'getName' in 'circt::calyx::InstanceOp'
    return getAttributeNameForIndex((*this)->getName(), index);
                                    ~~~~~~~  ^
include/circt/Dialect/Calyx/Calyx.h.inc:724:194: error: no member named 'CellInterface' in namespace 'circt::calyx'
class RegisterOp : public ::mlir::Op<RegisterOp, ::mlir::OpTrait::ZeroRegion, ::mlir::OpTrait::NResults<6>::Impl, ::mlir::OpTrait::ZeroSuccessor, ::mlir::OpTrait::ZeroOperands, ::circt::calyx::CellInterface::Trait, ::mlir::OpAsmOpInterface::Trait> {
                                                                                                                                                                                 ~~~~~~~~~~~~~~~~^
include/circt/Dialect/Calyx/Calyx.h.inc:726:9: error: use of undeclared identifier 'Op'
  using Op::Op;
        ^
include/circt/Dialect/Calyx/Calyx.h.inc:727:9: error: use of undeclared identifier 'Op'
  using Op::print;
        ^
include/circt/Dialect/Calyx/Calyx.h.inc:764:44: error: member reference type 'circt::calyx::RegisterOp' is not a pointer; did you mean to use '.'?
    return getAttributeNameForIndex((*this)->getName(), index);
                                    ~~~~~~~^~
                                           .
include/circt/Dialect/Calyx/Calyx.h.inc:764:46: error: no member named 'getName' in 'circt::calyx::RegisterOp'
    return getAttributeNameForIndex((*this)->getName(), index);
                                    ~~~~~~~  ^
../lib/Dialect/Calyx/CalyxOps.cpp:437:25: error: member reference type 'circt::calyx::InstanceOp' is not a pointer; did you mean to use '.'?
  auto program = (*this)->getParentOfType<ProgramOp>();
                 ~~~~~~~^~
... // it continues

I’m pretty sure line 1 is the important part:

include/circt/Dialect/Calyx/Calyx.h.inc:588:192: error: no member named 'CellInterface' in namespace 'circt::calyx'
class InstanceOp : public ::mlir::Op<InstanceOp, ::mlir::OpTrait::ZeroRegion, ::mlir::OpTrait::VariadicResults, ::mlir::OpTrait::ZeroSuccessor, ::mlir::OpTrait::ZeroOperands, ::circt::calyx::CellInterface::Trait> {

…but I’m unsure how to fix it. Doesn’t DeclareOpInterfaceMethods<...> declare for me?

How would you want to verify it? And just to be clear using TableGen’s operators you can have it define this extraClassDeclaration for you in the parent op class [now you can’t override extraClassDeclaration further then, and I do want to change that]. So these would be constructed for you.

Operation Definition Specification (ODS) - MLIR and it is used extensively TensorFlow side.

It doesn’t differ fundamentally, your portNames function is such a mapping. I was more clarifying if there is something subtle that requires it to be set. And similarly the OpInterface only makes it so that you can say cast<CellInterface>(op).portNames(), but where you store it doesn’t change.

1 Like