Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mlir][linalg] Create a dedicated target for LinalgRelayoutInterface #127541

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions mlir/include/mlir/Dialect/Linalg/IR/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ add_public_tablegen_target(MLIRLinalgRelayoutOpsIncGen)
add_dependencies(MLIRLinalgRelayoutOpsIncGen LinalgOdsGen)
add_dependencies(mlir-headers MLIRLinalgRelayoutOpsIncGen)

set(LLVM_TARGET_DEFINITIONS LinalgRelayoutInterface.td)
mlir_tablegen(LinalgRelayoutInterface.h.inc -gen-op-interface-decls)
mlir_tablegen(LinalgRelayoutInterface.cpp.inc -gen-op-interface-defs)
add_public_tablegen_target(MLIRLinalgRelayoutInterfaceIncGen)
add_dependencies(mlir-headers MLIRLinalgRelayoutInterfaceIncGen)

set(LLVM_TARGET_DEFINITIONS LinalgInterfaces.td)
mlir_tablegen(LinalgInterfaces.h.inc -gen-op-interface-decls)
mlir_tablegen(LinalgInterfaces.cpp.inc -gen-op-interface-defs)
Expand Down
3 changes: 3 additions & 0 deletions mlir/include/mlir/Dialect/Linalg/IR/LinalgInterfaces.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,7 @@ LogicalResult verifyStructuredOpInterface(Operation *op);
/// Include the generated interface declarations.
#include "mlir/Dialect/Linalg/IR/LinalgInterfaces.h.inc"

/// Include the generated relayout interface declarations.
#include "mlir/Dialect/Linalg/IR/LinalgRelayoutInterface.h.inc"

#endif // MLIR_DIALECT_LINALG_IR_LINALGINTERFACES_H_
10 changes: 0 additions & 10 deletions mlir/include/mlir/Dialect/Linalg/IR/LinalgInterfaces.td
Original file line number Diff line number Diff line change
Expand Up @@ -178,16 +178,6 @@ def LinalgConvolutionOpInterface : OpInterface<"ConvolutionOpInterface"> {
];
}

def LinalgRelayoutOpInterface : OpInterface<"RelayoutOpInterface"> {
let description = [{
A Linalg relayout-op is either linalg.pack or linalg.unpack.

While we could extend this interface with methods from Linalg_RelayoutOp,
this is currently not needed and left as a TODO.
}];
let cppNamespace = "::mlir::linalg";
}

def LinalgFillOpInterface : OpInterface<"FillOpInterface"> {
let description = [{
A fill operation is defined in general terms:
Expand Down
25 changes: 25 additions & 0 deletions mlir/include/mlir/Dialect/Linalg/IR/LinalgRelayoutInterface.td
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//===- LinalgInterfaces.td - Linalg Interfaces Declaration -*- 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 LINALG_IR_LINALGRELAYOUTINTERFACE
#define LINALG_IR_LINALGRELAYOUTINTERFACE

include "mlir/Interfaces/DestinationStyleOpInterface.td"
include "mlir/IR/OpBase.td"

def LinalgRelayoutOpInterface : OpInterface<"RelayoutOpInterface"> {
let description = [{
A Linalg relayout-op is either linalg.pack or linalg.unpack.

While we could extend this interface with methods from Linalg_RelayoutOp,
this is currently not needed and left as a TODO.
}];
let cppNamespace = "::mlir::linalg";
}

#endif // LINALG_IR_LINALGRELAYOUTINTERFACE
1 change: 1 addition & 0 deletions mlir/include/mlir/Dialect/Linalg/IR/LinalgRelayoutOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ include "mlir/Interfaces/DestinationStyleOpInterface.td"
include "mlir/Interfaces/SideEffectInterfaces.td"
include "mlir/Interfaces/InferTypeOpInterface.td"
include "mlir/Dialect/Linalg/IR/LinalgInterfaces.td"
include "mlir/Dialect/Linalg/IR/LinalgRelayoutInterface.td"
include "mlir/IR/OpAsmInterface.td"

//===----------------------------------------------------------------------===//
Expand Down
2 changes: 1 addition & 1 deletion mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "mlir/Dialect/Arith/IR/Arith.h"
#include "mlir/Dialect/Arith/Utils/Utils.h"
#include "mlir/Dialect/Complex/IR/Complex.h"
#include "mlir/Dialect/Linalg/IR/Linalg.h"
#include "mlir/Dialect/Linalg/IR/LinalgRelayoutInterface.h.inc"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer not to include .inc files from other dialects.

Let me try to create a separate bazel target for LinalgInterfaces.h, then we don't need any cmake changes and can include LinalgInterfaces.h instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, sorry I missed that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't include a .h.inc, but you can create a include "mlir/Dialect/Linalg/IR/LinalgRelayoutInterface.h" that itself encapsulates and include the .h.inc.
See for example include/mlir/Dialect/Bufferization/IR/AllocationOpInterface.h, include/mlir/Dialect/GPU/IR/CompilationInterfaces.h, or include/mlir/Conversion/ConvertToLLVM/ToLLVMInterface.h which implement this pattern.

#include "mlir/Dialect/Tensor/IR/Tensor.h"
#include "mlir/Dialect/Tensor/Utils/Utils.h"
#include "mlir/Dialect/Utils/IndexingUtils.h"
Expand Down