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

[TTNN dialect] Matmul op implemented as DPS, but isn't DPS in lib #1661

Closed
svuckovicTT opened this issue Dec 24, 2024 · 3 comments
Closed

[TTNN dialect] Matmul op implemented as DPS, but isn't DPS in lib #1661

svuckovicTT opened this issue Dec 24, 2024 · 3 comments
Labels
MLIR Ops Issues related to MLIR dialect ops and their implementations

Comments

@svuckovicTT
Copy link
Contributor

svuckovicTT commented Dec 24, 2024

In TTNN Dialect, MatmulOp is implemented as DPS op. However, there's a single signature in lib for ttnn::matmul and it doesn't support a dest arg. Dialect needs to be fixed.

Lib:

struct MatmulOperation {
    static Tensor invoke(
        const Tensor& input_tensor_a,
        const Tensor& input_tensor_b,
        const bool transpose_a = false,
        const bool transpose_b = false,
        const std::optional<const MemoryConfig>& memory_config = std::nullopt,
        const std::optional<const DataType> dtype = std::nullopt,
        const std::optional<const MatmulProgramConfig>& program_config = std::nullopt,
        const std::optional<const std::string>& activation = std::nullopt,
        const std::optional<const DeviceComputeKernelConfig> compute_kernel_config = std::nullopt,
        const std::optional<const CoreGrid> core_grid = std::nullopt,
        const std::optional<const tt::tt_metal::Tile>& output_tile = std::nullopt);
};

tabledef:

def TTNN_MatmulOp : TTNN_NamedDPSOp<"matmul"> {
    let arguments = (ins AnyRankedTensor:$a,
                         AnyRankedTensor:$b,
                         AnyRankedTensor:$output);
    let results = (outs AnyRankedTensor:$result);

    let extraClassDeclaration = [{
      MutableOperandRange getDpsInitsMutable() { return getOutputMutable(); }
    }];

    let hasVerifier = 1;
}
@svuckovicTT svuckovicTT added the MLIR Ops Issues related to MLIR dialect ops and their implementations label Dec 24, 2024
@svuckovicTT
Copy link
Contributor Author

fyi @sdjordjevicTT

@sdjordjevicTT
Copy link
Contributor

Hey, @svuckovicTT we created an issue on the metal side regarding this a month ago:
tenstorrent/tt-metal#15038

They recently fixed this, and probably with the next metal uplift, we will have matuml DPS version.

@svuckovicTT
Copy link
Contributor Author

That's awesome, confirmed it works for emitc mnist path, closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MLIR Ops Issues related to MLIR dialect ops and their implementations
Projects
None yet
Development

No branches or pull requests

2 participants