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

Enable remove-dead-values MLIR pass for stablehlo pipeline #926

Merged
merged 6 commits into from
Nov 5, 2024

Conversation

uazizTT
Copy link
Contributor

@uazizTT uazizTT commented Oct 16, 2024

Enable both remove-dead-values pass by default.

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.

Minor nit otherwise looks good

@mrakitaTT
Copy link
Contributor

Btw just curious, can you please post some example StableHLO graph (or resulting TTIR graph) where you've found the sccp optimization useful?

@uazizTT uazizTT force-pushed the uaziz/stablehlo-sccp-pass branch from add55ca to c058073 Compare October 18, 2024 02:26
@uazizTT
Copy link
Contributor Author

uazizTT commented Oct 22, 2024

Btw just curious, can you please post some example StableHLO graph (or resulting TTIR graph) where you've found the sccp optimization useful?

I found a couple of opportunities in a Llama model where SCCP will be beneficial, so I created a MLIR test from it, but it seems that SCCP pass is not working even for a simple case. We can merge this patch to have a placeholder in case someone is looking into it in future or else can hold it until me or someone else can figure out why SCCP is not working.

@nsmithtt
Copy link
Contributor

Btw just curious, can you please post some example StableHLO graph (or resulting TTIR graph) where you've found the sccp optimization useful?

I found a couple of opportunities in a Llama model where SCCP will be beneficial, so I created a MLIR test from it, but it seems that SCCP pass is not working even for a simple case. We can merge this patch to have a placeholder in case someone is looking into it in future or else can hold it until me or someone else can figure out why SCCP is not working.

So is it used/working for any case that we have right now? I'd prefer to just do this work before landing this PR unless this PR is blocking something else. We should strive to land it in a working state.

@uazizTT uazizTT force-pushed the uaziz/stablehlo-sccp-pass branch from c47e811 to 65db76a Compare November 4, 2024 20:17
@uazizTT uazizTT changed the title Enable MLIR passes for stablehlo pipeline Enable remove-dead-values MLIR pass for stablehlo pipeline Nov 4, 2024
@uazizTT uazizTT merged commit 3883db6 into main Nov 5, 2024
18 checks passed
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