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

[Minor] Negative dim support for concat #651

Merged
merged 5 commits into from
Sep 10, 2024

Conversation

mtopalovicTT
Copy link
Contributor

@mtopalovicTT mtopalovicTT commented Sep 8, 2024

  • Torch supports it. Forge passes negative dim attr for concat.
  • Added more positive and negative tests.

Torch supports it. Forge passes negative dim attr for concat.
@mtopalovicTT mtopalovicTT changed the title [Minor] Adding negative dim support for concat [Minor] Negative dim support for concat Sep 9, 2024
include/ttmlir/Dialect/TTIR/IR/TTIROps.td Outdated Show resolved Hide resolved
@mtopalovicTT mtopalovicTT merged commit 3f52ccc into main Sep 10, 2024
11 checks passed
@@ -173,9 +173,14 @@ class ConcatOpConversionPattern : public OpConversionPattern<ttir::ConcatOp> {
LogicalResult
matchAndRewrite(ttir::ConcatOp op, OpAdaptor adaptor,
ConversionPatternRewriter &rewriter) const override {
int dim = adaptor.getDim();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you elaborate on why we convert from negative to positive dim in TTIR to TTNN conversion? Do we want both dialects to support the negative dim?

uazizTT pushed a commit that referenced this pull request Sep 12, 2024
* Adding negative dim support for `concat`

Torch supports it. Forge passes negative dim attr for concat.

* Renaming dim to axis to align with tt-forge attr name

* Revert dim attr rename

* Remove test
@mtopalovicTT mtopalovicTT deleted the milant/concat_negative_dim_support branch December 31, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants