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

Select op support #1338

Merged
merged 4 commits into from
Nov 21, 2024
Merged

Select op support #1338

merged 4 commits into from
Nov 21, 2024

Conversation

rpavlovicTT
Copy link
Contributor

  • Add select op to TTIR
  • Decompose select op to TTIR slices and concat optionally
  • Unit tests (negative and positive)

Fixes #1179

@rpavlovicTT rpavlovicTT added the MLIR Ops Issues related to MLIR dialect ops and their implementations label Nov 20, 2024
@rpavlovicTT rpavlovicTT self-assigned this Nov 20, 2024
lib/Dialect/TTIR/IR/TTIROps.cpp Outdated Show resolved Hide resolved
include/ttmlir/Dialect/TTIR/IR/TTIROps.td Outdated Show resolved Hide resolved
include/ttmlir/Dialect/TTIR/IR/TTIROps.td Outdated Show resolved Hide resolved
lib/Dialect/TTIR/IR/TTIROps.cpp Outdated Show resolved Hide resolved
lib/Dialect/TTIR/IR/TTIROps.cpp Show resolved Hide resolved
* Add select op to TTIR
* Decompose select op to TTIR slices and concat optionally
* Unit tests
@mrakitaTT
Copy link
Contributor

Shouldn't this op be called TTIR.SliceOp? Is it called Select in Forge? Select and Where are used in other dialects for the op that selects values from tensor based on predicate (examples: stablehlo, tosa, onnx). In TTIR we used Where, so using Select for slice op could lead to confusion.

@rpavlovicTT rpavlovicTT merged commit e78bb00 into main Nov 21, 2024
18 checks passed
@rpavlovicTT
Copy link
Contributor Author

Shouldn't this op be called TTIR.SliceOp? Is it called Select in Forge? Select and Where are used in other dialects for the op that selects values from tensor based on predicate (examples: stablehlo, tosa, onnx). In TTIR we used Where, so using Select for slice op could lead to confusion.

This select is based on forge select which is not the as select in other dialects, nor it is the same as ttir.slice op. Note that it can be decomposed to many slice ops, but it's not 1:1. Based on the wide/narrow TTIR dialect design, I thought this made sense to copy select op from ForgeFE and decompose it as possible. If you have more concerns, we can discuss further offline.

@mrakitaTT
Copy link
Contributor

Talked offline, seems like we just need a better name for this op. @nvukobratTT is it okay to change op names in Forge? Did you manually choose this name (Select) or was it chosen based on TVM or something?

@nvukobratTT
Copy link
Contributor

Talked offline, seems like we just need a better name for this op. @nvukobratTT is it okay to change op names in Forge? Did you manually choose this name (Select) or was it chosen based on TVM or something?

It's okay to change op names in Forge, if we figure out that other is better suited :))

Most of the ops on Forge are inspired by TVM. However, I wouldn't limit us to use those names, if we find ones that are more suitable and aligned with TTIR and other frontends.

@mrakitaTT @rpavlovicTT I don't have full context, so feel free to send you proposal with a bit more details :))

jserbedzijaTT pushed a commit that referenced this pull request Nov 25, 2024
Select op support

* Add select op to TTIR
* Decompose select op to TTIR slices and concat optionally
* Unit tests
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 this pull request may close these issues.

[Ops] Support for select op (ttnn.slice)
7 participants