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

Merge AMD-HIP port #1031

Open
VincentSC opened this issue May 15, 2017 · 8 comments
Open

Merge AMD-HIP port #1031

VincentSC opened this issue May 15, 2017 · 8 comments

Comments

@VincentSC
Copy link

AMD has just released their port for Torch7 to AMD GPUs, to be found here.
Please merge that code, so Torch7 works on both NVidia and AMD hardware.

@pavanky
Copy link
Contributor

pavanky commented May 15, 2017

This will need to be sent in as a PR by AMD. I am not sure what opening an issue for this accomplishes.

@VincentSC
Copy link
Author

For discussion purposes mostly, before AMD comes with that big PR.

@pavanky
Copy link
Contributor

pavanky commented May 15, 2017

Then perhaps change the title and description to say that?

@soumith
Copy link
Member

soumith commented May 15, 2017

thanks a lot @VincentSC
We have to think through how we abstract devices. Right now I think having hiptorch as a separate package, similar to how we have cutorch as a similar package makes sense.
We are working on refactoring internals to figure out the right device abstractions.

@wickedfoo
Copy link
Contributor

Having a HIP version be separate from cutorch I think would only encourage code rot (on the HIP side of things), as cutorch changes fairly frequently. I wonder to what extent the HIP port has kept up with changes in cutorch?

As a standalone package, it certainly would be much easier, but who on our end would be responsible for keeping the HIP port up to data with functionality additions and changes?

@wickedfoo
Copy link
Contributor

unless we can factor out the kernel side of things from the glue/boilerplate, in which case CUDA and HIP packages make sense. The ideal solution would have an easy way to compare the CUDA and HIP implementations of a particular kernel, so one doesn't have to go chasing for differences.

@soumith
Copy link
Member

soumith commented May 15, 2017

yea, we are figuring out the refactor, and introducing a proper device-abstraction.

@VincentSC
Copy link
Author

@soumith Some background. HIP was designed to avoid code rot (as @wickedfoo mentions) - OpenCL has had this problem when added to CUDA, I've seen in several cases. As HIP is a subset of CUDA, it could be handled as there is a Kepler-optimizes and Pascal-optimized branch - large parts can be shared code while some parts are ifdef'ed.
Do feel free to ask me anything on HIP, hcc and AMD's other new libraries and compilers.

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

No branches or pull requests

4 participants