-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[AMD] Remove tritonamdgpu-stream-pipeline #4845
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay! Just one final nit.
third_party/amd/backend/compiler.py
Outdated
else: | ||
if options.num_stages == 0: | ||
amd.passes.ttgpuir.add_stream_pipeline(pm) | ||
num_stages = options.num_stages if options.num_stages != 0 else 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also drop this too--it was meant to smooth transition. After v1 gone we need to respect what the user set--if it's 0 then we don't pipeline anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will have a big influence on almost everyone. For now, all gemm-like kernels on AMD backend uses num_stages=0. Removing it will cause a significant perf regression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah num_stages=0
just need to be fixed in all downstream users--we cannot be beholden to that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just blocking for now to still wait for sometime for stability of v2 before landing.
No description provided.