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

[rocm-opencl-runtime] add support for gfx800s #741

Closed
John-Gee opened this issue Apr 6, 2022 · 9 comments
Closed

[rocm-opencl-runtime] add support for gfx800s #741

John-Gee opened this issue Apr 6, 2022 · 9 comments
Labels
enhancement New feature or request patched

Comments

@John-Gee
Copy link
Contributor

John-Gee commented Apr 6, 2022

Hello,

Until AMD accepts to readd Polaris support, would it be acceptable to add the patch from ROCm/ROCm#1659 to do so?

Thank you!

rel com: ROCm/ROCm#1659 (comment)

@acxz acxz changed the title Readd support for Polaris [rocm-opencl-runtime] add support for gfx800s (Polaris) Apr 6, 2022
@acxz
Copy link
Member

acxz commented Apr 6, 2022

Usually we try our best to maintain as close to upstream as possible so that issues that show up for users on Arch can be considered valid issues for upstream ROCm repos. i.e. I don't want users facing issues here complaining to ROCm for what we have done uniquely. That creates a bad image for archlinux users to the AMD folks, which I would like to avoid.

That being said, reading through the issue it seems that there is quite a lot of support for this and the patch itself is just switching a boolean. In lieu of this, I would support a PR that adds support for gfx8** cards.

@acxz acxz added the enhancement New feature or request label Apr 6, 2022
@John-Gee
Copy link
Contributor Author

John-Gee commented Apr 6, 2022

It's definitely a fair rule to keep as close as possible to upstream, but as you wrote it's just a boolean, so for those that didn't have it working it shouldn't be much worse, and hopefully it won't impact others.

If you're ok with a PR, I'll create one then. Thanks!

@acxz acxz changed the title [rocm-opencl-runtime] add support for gfx800s (Polaris) [rocm-opencl-runtime] add support for gfx800s Apr 6, 2022
@John-Gee
Copy link
Contributor Author

John-Gee commented Apr 9, 2022

Done with #742

Thank you!

@John-Gee John-Gee closed this as completed Apr 9, 2022
@acxz
Copy link
Member

acxz commented Apr 9, 2022

@John-Gee we keep our issues open that are patched, so we know we are different from AMD.

@acxz acxz reopened this Apr 9, 2022
@acxz acxz added the patched label Apr 9, 2022
@John-Gee
Copy link
Contributor Author

John-Gee commented Apr 9, 2022

My apologies!

@tpkessler tpkessler removed the enhancement New feature or request label Apr 9, 2022
@acxz
Copy link
Member

acxz commented Apr 10, 2022

@tpkessler is there any reason why the enhancement label was removed?

@tpkessler
Copy link
Member

My reasoning was that an enhancement cannot be patched :)

@acxz
Copy link
Member

acxz commented Apr 10, 2022

I think I would consider this a patch since it changes the AMD codebase, as well as an enhancement as we are adding a feature that AMD is not.

It's a feature patch ;)

@John-Gee
Copy link
Contributor Author

John-Gee commented Sep 10, 2022

So it seems the patch may not be needed anymore, the variable ROC_ENABLE_PRE_VEGA=1 can do it instead. I verified that it works on my computer but only rebuilt this package. I'll create another PR to undo this and then maybe add the info to the wiki?

Thanks!

edit: created #853

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request patched
Projects
None yet
Development

No branches or pull requests

3 participants