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

[LinalgExt] Retire LinalgExt::ReverseOp #17866

Merged
merged 3 commits into from
Jul 19, 2024
Merged

[LinalgExt] Retire LinalgExt::ReverseOp #17866

merged 3 commits into from
Jul 19, 2024

Conversation

lialan
Copy link
Contributor

@lialan lialan commented Jul 11, 2024 •
edited by hanhanW
Loading

LinalgExt::ReverseOp is only lowered from stablehlo::ReverseOp. We can expand stablehlo::ReverseOp to a different pattern and retire LinalgExt::ReverseOp.

Fixes #16060

@lialan lialan force-pushed the lialan/linalgext_reverse branch 4 times, most recently from fffdf8a to f24b4d8 Compare July 12, 2024 00:09
@lialan lialan linked an issue Jul 12, 2024 that may be closed by this pull request
Perhaps we can retire linalg_ext.reverse op? #16060
Closed
@lialan lialan force-pushed the lialan/linalgext_reverse branch 2 times, most recently from f041fdf to cf53dfd Compare July 12, 2024 00:56
@lialan lialan marked this pull request as ready for review July 12, 2024 01:39
@hanhanW hanhanW added benchmarks:cuda Run default CUDA benchmarks benchmarks:x86_64 Run default x86_64 benchmarks benchmarks:comp-stats Run default compilation statistics benchmarks benchmarks:android-cpu Run default Android CPU benchmarks benchmarks:android-gpu Run default Android GPU benchmarks benchmarks:vulkan-nvidia Run default Vulkan benchmarks on NVIDIA GPU labels Jul 15, 2024
hanhanW
hanhanW requested changes Jul 15, 2024
View reviewed changes
Copy link
Contributor

@hanhanW hanhanW left a comment

Choose a reason for hiding this comment

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

Nice cleanup, thanks! Just few nits.

compiler/plugins/input/StableHLO/Conversion/StableHLOToLinalgExt.cpp Outdated Show resolved Hide resolved
compiler/plugins/input/StableHLO/Conversion/StableHLOToLinalgExt.cpp Outdated Show resolved Hide resolved
compiler/plugins/input/StableHLO/Conversion/StableHLOToLinalgExt.cpp Outdated Show resolved Hide resolved
compiler/plugins/input/StableHLO/Conversion/StableHLOToLinalgExt.cpp Outdated Show resolved Hide resolved
@hanhanW hanhanW changed the title Retire LinalgExt::ReverseOp [LinalgExt] Retire LinalgExt::ReverseOp Jul 15, 2024
@lialan lialan force-pushed the lialan/linalgext_reverse branch 4 times, most recently from 268f829 to 10193a9 Compare July 16, 2024 04:20
@lialan lialan requested a review from hanhanW July 16, 2024 13:19
hanhanW
hanhanW approved these changes Jul 16, 2024
View reviewed changes
Copy link
Contributor

@hanhanW hanhanW left a comment

Choose a reason for hiding this comment

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

LGTM, just few style nits. Thanks!

compiler/plugins/input/StableHLO/Conversion/StableHLOToLinalgExt.cpp Outdated Show resolved Hide resolved
compiler/plugins/input/StableHLO/Conversion/StableHLOToLinalgExt.cpp Outdated Show resolved Hide resolved
compiler/plugins/input/StableHLO/Conversion/StableHLOToLinalgExt.cpp Outdated Show resolved Hide resolved
@lialan lialan enabled auto-merge (squash) July 16, 2024 18:46
@lialan lialan removed the benchmarks:cuda Run default CUDA benchmarks label Jul 18, 2024
@lialan lialan removed benchmarks:x86_64 Run default x86_64 benchmarks benchmarks:comp-stats Run default compilation statistics benchmarks benchmarks:android-cpu Run default Android CPU benchmarks benchmarks:android-gpu Run default Android GPU benchmarks benchmarks:vulkan-nvidia Run default Vulkan benchmarks on NVIDIA GPU labels Jul 18, 2024
@lialan
Retire LinalgExt::ReverseOp
4d5e5f0 
Signed-off-by: Alan Li <me@alanli.org>
@lialan
Remove linalgExt::ReverseOp and its tests
b87d1a1 
Signed-off-by: Alan Li <me@alanli.org>
@lialan
Update according to comments.
6524f19 
Signed-off-by: Alan Li <me@alanli.org>
@lialan lialan disabled auto-merge July 18, 2024 03:27
@ScottTodd
Copy link
Member

Want this merged? I cancelled the job that was stalled waiting for a runner.

@lialan
Copy link
Contributor Author

lialan commented Jul 19, 2024

Want this merged? I cancelled the job that was stalled waiting for a runner.

@ScottTodd Please merge it. Thank you.

@ScottTodd ScottTodd merged commit 76cad82 into iree-org:main Jul 19, 2024
53 of 54 checks passed
@ScottTodd ScottTodd mentioned this pull request Jul 19, 2024
Open
@lialan lialan deleted the lialan/linalgext_reverse branch July 19, 2024 17:05
LLITCHEV pushed a commit to LLITCHEV/iree that referenced this pull request Jul 30, 2024
@lialan @LLITCHEV
[LinalgExt] Retire LinalgExt::ReverseOp (iree-org#17866)
b87561b 
`LinalgExt::ReverseOp` is only lowered from `stablehlo::ReverseOp`. We
can expand `stablehlo::ReverseOp` to a different pattern and retire
`LinalgExt::ReverseOp`.

Fixes iree-org#16060

---------

Signed-off-by: Alan Li <me@alanli.org>
Signed-off-by: Lubo Litchev <lubol@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hanhanW hanhanW hanhanW approved these changes

@MaheshRavishankar MaheshRavishankar Awaiting requested review from MaheshRavishankar MaheshRavishankar is a code owner

@rsuderman rsuderman Awaiting requested review from rsuderman rsuderman is a code owner

@benvanik benvanik Awaiting requested review from benvanik benvanik is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Perhaps we can retire linalg_ext.reverse op?
3 participants
@lialan @ScottTodd @hanhanW