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] Conv2d op missing some optional parameters #1681

Open
odjuricicTT opened this issue Dec 30, 2024 · 3 comments · May be fixed by #2054
Open

[TTNN Dialect] Conv2d op missing some optional parameters #1681

odjuricicTT opened this issue Dec 30, 2024 · 3 comments · May be fixed by #2054
Assignees
Labels
MLIR Ops Issues related to MLIR dialect ops and their implementations

Comments

@odjuricicTT
Copy link
Contributor

Currently Conv2dOp in TTNN IR is missing a few arguments from ttnn conv2d invoke function.
Mainly Conv2dConfig, DeviceComputeKernelConfig and MemoryConfig (related #620).

TTNN IR Conv2dOp should implement all these arguments to be exactly the same as in the actual op call:

struct Conv2dOperation{
    static Result invoke(
        uint8_t queue_id,
        const ttnn::Tensor& input_tensor,
        const ttnn::Tensor& weight_tensor,
        Device * device,
        uint32_t in_channels,
        uint32_t out_channels,
        uint32_t batch_size,
        uint32_t input_height,
        uint32_t input_width,
        std::array<uint32_t, 2> kernel_size,
        std::array<uint32_t, 2> stride,
        std::array<uint32_t, 2> padding,
        std::array<uint32_t, 2> dilation,
        uint32_t groups,
        std::optional<const ttnn::Tensor> bias_tensor = std::nullopt,
        const std::optional<const Conv2dConfig>& conv_config_ = std::nullopt,
        const std::optional<const DeviceComputeKernelConfig>& compute_config_ = std::nullopt,
        const std::optional<const MemoryConfig>& memory_config = std::nullopt);

https://github.com/tenstorrent/tt-metal/blob/main/ttnn/cpp/ttnn/operations/conv/conv2d/conv2d.hpp#L44

@odjuricicTT odjuricicTT changed the title Conv2d op missing some optional parameters [TTNN Dialect] Conv2d op missing some optional parameters Dec 30, 2024
@odjuricicTT odjuricicTT added this to the [Performance - 2] March milestone Dec 30, 2024
@odjuricicTT
Copy link
Contributor Author

@sdjordjevicTT Do you think that someone from your team can take a look after the holidays?

@sdjordjevicTT
Copy link
Contributor

Thanks for reporting this @odjuricicTT, @jserbedzijaTT will pick this up after vacation.

@sdjordjevicTT sdjordjevicTT added the MLIR Ops Issues related to MLIR dialect ops and their implementations label Jan 9, 2025
@odjuricicTT
Copy link
Contributor Author

Great, i think that this should be staged in a way so that Conv2dConfig is added first. It's the most important for optimizer usecase.

MemoryConfig is a bit more compilcated and required further discussion.

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

Successfully merging a pull request may close this issue.

3 participants