Wrong : Add a new linalg op

MLIR
1

I want to add a new linalg op, the code is as follows. but now I got a problem.
when I use

isa<linalg::LinalgOp>(op)

to determine whether it is a linalg op ，and I got false. I am very confused.

def Linalg_AvgPool2DOp : Linalg_Op<“avg_pool_2d”, [NoSideEffect]>{
let summary = “2D linalg avg pool operation”;
let description = [{
2D avg pool operation
}];

let arguments = (ins AnyShaped:$input,
AnyShaped:$output,
I64ElementsAttr:$window_dimensions,
OptionalAttr:$padding,
OptionalAttr:$window_strides,
OptionalAttr:$base_dilations,
OptionalAttr:$window_dilations,
OptionalAttr:$library_call);

let results = (outs AnyShaped:$result);

let printer = ?;
let verifier = ?;
let parser = ?;

}

2

That is confusing. Could you share a patch where you’re seeing this behavior? From what you’ve got here it seems this should indeed be a linalg op and I suspect there’s some other issue with code or understanding that’s causing you to see this result.

3

If I had to take a guess, the type of op is linalg.avg_pool_2d . In that case you will need isa<linalg::LinalgOp>(op.getOperation()). I actually dont know why that is the case.

4

This op is a new op I added and all the code are as shown above. I add the code to LinalgOps.td and don’t modify others files

5

Unfortunately，I do isa<linalg::LinalgOp>(op.getOperation()) . but I still got false :sob: :sob:

6

You can start by inspecting a bit more, can you try to print this and post the results:

llvm::errs() << op->getName() << " : " << op->getAbstractOperation() <<  " : " << isa<linalg::LinalgOp>(op.getOperation())  << "\n";

You can also check in the build directory for tools/mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.cpp.inc ; it starts with a list that looks like:

#ifdef GET_OP_LIST
#undef GET_OP_LIST

::mlir::linalg::BatchMatmulI16I16I32Op,
::mlir::linalg::BatchMatmulI32I32I32Op,
::mlir::linalg::BatchMatmulI8I8I32Op,
::mlir::linalg::BatchMatmulOp,
...

Do you find your new op in this list?

Next step after this would be to post a patch on Phabricator that reproduces your issue.

7

did you mean that my new linalg op should inherit LinalgStructuredBase_Op， not Linalg_Op?

8

Yes I think you need to derive from LinalStructuredBase_Op.

I would recommend to start from an existing operation such as the CopyOp when implementing a new one. Also consider if you want to use the OpDSL to define your own operation. It allows you to specify the operation in Python (Linalg OpDSL - MLIR) and translate it to a C++ operation via a yaml configuration file.

9

Not directly related to the question, but I’d like to point out that I would recommend NOT to add a such op to Linalg in this way, either to upstream or local. The padding attributes will stop many opportunities from Linalg Transform, e.g., tile+fusion. There are couple options to support the op:

  1. You can use a linalg.pooling_sum_* + div (or a linalg.generic) to represent the op in Linalg.
  2. Add a named op to https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/Linalg/IR/LinalgNamedStructuredOps.yaml

For padding attributes, you can lower it to linalg.pad_tensor and pass the result to Linalg structured ops.