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.

Sign up for GitHub

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

Jump to bottom

Change ttir -> ttnn to use dialect conversion API #221

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Change ttir -> ttnn to use dialect conversion API #221

merged 1 commit into from
Jul 25, 2024

Conversation

odjuricicTT
Copy link
Contributor

@odjuricicTT odjuricicTT commented Jul 24, 2024 •
edited
Loading

Details:

This leaves the repo in an inconsistent state since the current ttnn->emitc conversion pass follows torch-mlir structure.

Next steps:

  • Refactor common conversion pass files from torch approach to llvm
  • Refactor ttnn -> emitc to llvm approach
  • Add tests

@odjuricicTT odjuricicTT requested a review from svuckovicTT July 24, 2024 11:47
@nobradovictt
Copy link
Contributor

Which tests need to be added?

@nobradovictt
Copy link
Contributor

Looks great, lets do this for emitc as well.

@odjuricicTT
Copy link
Contributor Author

odjuricicTT commented Jul 24, 2024 •
edited
Loading

Which tests need to be added?

I guess it's already covered with the pipeline tests.

svuckovicTT
svuckovicTT reviewed Jul 24, 2024
View reviewed changes
lib/Conversion/TTIRToTTNN/TTIRToTTNNPass.cpp
RewritePatternSet patterns(&getContext());
populateTTIRToTTNNPatterns(&getContext(), patterns, typeConverter);

if (failed(applyPartialConversion(getOperation(), target,
Copy link
Contributor

Choose a reason for hiding this comment

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

Leave comment on why partial conversion.

odjuricicTT
odjuricicTT commented Jul 24, 2024
View reviewed changes
lib/Conversion/TTIRToTTNN/TTIRToTTNN.cpp Outdated
// ANCHOR: adding_an_op_matmul_op_rewriter
template <typename TTIROp, typename TTNNOp,
typename OpAdaptor = typename TTIROp::Adaptor>
class BinaryOpConversionPattern : public OpConversionPattern<TTIROp> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nsmithtt I took the name from the previous implementation, but i'm a bit confused as to why is matmul is binary op? In the .td definition it has 3 arguments (two inputs and a bias?), where as elementwise binary ops have 2.

Copy link
Contributor

Choose a reason for hiding this comment

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

The third param is the output (due to DPS). However, I'm wondering why we don't have a matmul with bias - is it just for simplicity in TTIR? I understand that we can fuse them in TTNN for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

It was here just to simplify the add e2e op tutorial, but I agree matmul should get its own pattern and eventually we should add the optional bias param.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, will rename the pattern to MatmulOpConversionPattern until there is a need for more abstraction.

@odjuricicTT odjuricicTT mentioned this pull request Jul 24, 2024
Closed
nsmithtt
nsmithtt reviewed Jul 24, 2024
View reviewed changes
docs/src/adding-an-op.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for keeping the docs up to date!

nsmithtt
nsmithtt approved these changes Jul 24, 2024
View reviewed changes
Copy link
Contributor

@nsmithtt nsmithtt left a comment

Choose a reason for hiding this comment

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

Looks great :)

@odjuricicTT odjuricicTT force-pushed the odjuricic/ttir-to-ttnn-dialect-conversion branch from 254888a to a5ea756 Compare July 25, 2024 07:24
@odjuricicTT odjuricicTT force-pushed the odjuricic/ttir-to-ttnn-dialect-conversion branch from 23fa844 to 96e8288 Compare July 25, 2024 07:53
@odjuricicTT odjuricicTT merged commit 01c19bd into main Jul 25, 2024
4 checks passed
vprajapati-tt added a commit that referenced this pull request Jul 29, 2024
@vprajapati-tt
Squashed commit of the following:
7317357 
commit 5ebfe37
Author: Sasa Vuckovic <134393361+svuckovicTT@users.noreply.github.com>
Date:   Mon Jul 29 16:42:29 2024 +0200

    Small refactor to ttnn-to-emitc pass (#229)

    * small refactor to ttnn-to-emitc pass

    * Remove unused constructor in DefaultOpConversionPattern

    * run pre-commit hooks that didn't get picked up

    * remove PassDetail.h

commit 60ad1a8
Author: Usman Aziz <uaziz@tenstorrent.com>
Date:   Mon Jul 29 10:29:46 2024 -0400

    Update flatbuffers dependency to allow bezel builds without python dependency. (#233)

commit 47a54b1
Author: Nikola Obradovic <132568163+nobradovictt@users.noreply.github.com>
Date:   Mon Jul 29 12:45:28 2024 +0200

    [Optimizer] Setting up rough shape of optimizer pass. Expanding analysis. (#236)

commit 29e618e
Author: Nick Smith <127986401+nsmithtt@users.noreply.github.com>
Date:   Sat Jul 27 15:24:27 2024 -0700

    embed sdk in ccache (#237)

commit 91e6b15
Author: Nick Smith <127986401+nsmithtt@users.noreply.github.com>
Date:   Fri Jul 26 19:51:25 2024 -0700

    Fix mac build; link against MLIR dylib (#239)

commit d817a8f
Author: Radenko Pavlovic <133032400+rpavlovicTT@users.noreply.github.com>
Date:   Fri Jul 26 14:00:45 2024 +0200

    Change TOSA->TTIR to use dialect conversion API (#230)

    Fixes #178

    Added conversion lib that implements conversion from TOSA to TTIR.
    Did not add new coverage of TOSA ops, it still supports only add, mul,
    sub, geq ops. Type conversion is still a no-op.

    Removed old conversion.

    Added MLIRTTConversions lib that links all conversion libs and is linked
    against TTMLIR.

commit 39ca8e2
Author: Nikola Obradovic <132568163+nobradovictt@users.noreply.github.com>
Date:   Fri Jul 26 09:15:16 2024 +0200

    [Misc] Rename TTIR op interface to TTIROp. (#227)

commit 01c19bd
Author: Ognjen Djuricic <160603639+odjuricicTT@users.noreply.github.com>
Date:   Thu Jul 25 09:54:25 2024 +0200

    Change ttir->ttnn to use dialect conversion api (#177) (#221)

commit 1fcda08
Author: Nick Smith <127986401+nsmithtt@users.noreply.github.com>
Date:   Wed Jul 24 12:12:42 2024 -0700

    Add `tt.device` spec #91 (#213)

commit a2f8e01
Author: Predrag Ilkic <148892209+pilkicTT@users.noreply.github.com>
Date:   Wed Jul 24 19:49:25 2024 +0200

    [docs] fix build on linux (#224)

    It looks like the `cp -r` command behaves differently on linux vs macOS.

    `cp -r dir/ new_dir/` will copy contents of the `dir/` into `new_dir/`
    on macOS, but on Linux, it will copy the directory, so you would end
    up with `new_dir/dir/`.

    Hence, changing the command to be:
    `cp -r dir/* new_dir/` which works the same on both platforms.

    Updating the docs for building docs as well.

commit 7345e48
Author: Milan Topalovic <163355844+mtopalovicTT@users.noreply.github.com>
Date:   Wed Jul 24 12:11:54 2024 +0200

    Adding translation from ttnn to flatbuffer (#216)

    We can now use ttmlir-translate tool to translate from TTNN IR to flatbuffer. Example:

    `./build/bin/ttmlir-opt --ttir-to-ttnn-backend-pipeline test/ttmlir/Dialect/TTNN/simple_matmul.mlir | ./build/bin/ttmlir-translate --ttnn-to-flatbuffer -o out.ttnn`

    To get flatbuffer from tt-forge we call std::vector<uint8_t> ttnnToFlatbuffer(Operation *op).

    resolves #120

commit 5cc95a4
Author: Stefan Djordjevic <157365107+sdjordjevicTT@users.noreply.github.com>
Date:   Wed Jul 24 10:25:56 2024 +0200

    Rewriting pipelines and transformation passes headers in LLVM style (#219)

commit a46f4f1
Author: Predrag Ilkic <148892209+pilkicTT@users.noreply.github.com>
Date:   Tue Jul 23 18:14:09 2024 +0200

    [tensor-layout] implements LayoutAttr::getStride() (#185)

    Initial implementation of stride calculation from the logical shape
    and the `LayoutAttr`. The strides of the tensors are stored in the
    Flatbuffer and are used by FE to allocate the tensors.

commit 6841e4d
Author: Nikola Obradovic <132568163+nobradovictt@users.noreply.github.com>
Date:   Tue Jul 23 09:50:31 2024 +0200

    [Optimizer] Per op grid overrides. (#206)

commit 2574010
Author: Radenko Pavlovic <133032400+rpavlovicTT@users.noreply.github.com>
Date:   Tue Jul 23 09:39:15 2024 +0200

    Eltwise interface and builders (#214)

    Fixes #110

commit f2c8b0b
Author: Jackson Nie <jnie@tenstorrent.com>
Date:   Mon Jul 22 16:02:14 2024 -0400

    Add runtime gtest and linker fix (#209)

    * Initial cmake changes for runtime gtest infra

    * Update instantiation of TTNN_LIBRARY

    * Fix linker error on fresh clone

commit ccbcfbf
Author: Nick Smith <127986401+nsmithtt@users.noreply.github.com>
Date:   Mon Jul 22 06:58:15 2024 -0700

    Debugging python (#211)

commit 25bb6ae
Author: Vladimir Milosevic <157983820+vmilosevic@users.noreply.github.com>
Date:   Mon Jul 22 10:21:20 2024 +0200

    Group workflows (#205)

    Organize workflows by grouping them in the parent workflow

    - adding top-level workflow 'on-pr-and-push-to-main' to group workflows
    - adding workflow dispatch to workflows to enable manual runs
    - added runtime on/off to cache key
    - update action versions
    - refactor build.yml matrix to dictionary

commit 704905a
Author: Muhammad Asif Manzoor <mmanzoor@tenstorrent.com>
Date:   Fri Jul 19 11:14:14 2024 -0400

    Add greater or equal op end to end (#200)

commit fc4f4ac
Author: Vladimir Milosevic <157983820+vmilosevic@users.noreply.github.com>
Date:   Fri Jul 19 09:52:54 2024 +0200

    Show and upload test report (#195)

    Display test summary and upload artifacts

commit 4fdde56
Author: Nick Smith <127986401+nsmithtt@users.noreply.github.com>
Date:   Thu Jul 18 12:36:53 2024 -0700

    Add Code of Conduct (#199)

commit 107fd54
Author: Jackson Nie <jnie@tenstorrent.com>
Date:   Thu Jul 18 15:09:56 2024 -0400

    Revert "Runtime gtest infrastructure (#122)" (#198)

    This reverts commit 1a581df.

commit 1a581df
Author: Jackson Nie <jnie@tenstorrent.com>
Date:   Thu Jul 18 13:53:32 2024 -0400

    Runtime gtest infrastructure (#122)

    * Initial cmake changes for runtime gtest infra

    * Update instantiation of TTNN_LIBRARY
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@svuckovicTT svuckovicTT svuckovicTT approved these changes

@nsmithtt nsmithtt nsmithtt approved these changes

@nobradovictt nobradovictt nobradovictt approved these changes

@sdjordjevicTT sdjordjevicTT sdjordjevicTT approved these changes

@rpavlovicTT rpavlovicTT Awaiting requested review from rpavlovicTT

Assignees

@odjuricicTT odjuricicTT

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@odjuricicTT @nobradovictt @nsmithtt @svuckovicTT @sdjordjevicTT