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

full OpenCL C? #63

Closed
tomeuv opened this Issue Sep 29, 2017 · 9 comments

Comments

Projects
None yet
4 participants
@tomeuv

tomeuv commented Sep 29, 2017

Would you accept patches to reach full OpenCL C compliance? Or is that a clear non-goal of the project?

@sheredom

This comment has been minimized.

Show comment
Hide comment
@sheredom

sheredom Sep 29, 2017

Contributor

I'll leave it to @dneto0 to have the final say, but from my perspective - I think it'd be great to implement all of OpenCL C (or at least as close to 100% as we could get).

The list of restrictions in https://github.com/google/clspv/blob/master/docs/OpenCLCOnVulkan.md was more to do with the set of kernels we had to get ported from OpenCL C to Vulkan SPIR-V, not necessarily things that couldn't be implemented.

For example, you could implement the vector of 8/16 elements types by using Vulkan matrices or a struct of vectors, but given the kernels we were targeting did not have this pattern it wasn't worth the engineering effort.

Contributor

sheredom commented Sep 29, 2017

I'll leave it to @dneto0 to have the final say, but from my perspective - I think it'd be great to implement all of OpenCL C (or at least as close to 100% as we could get).

The list of restrictions in https://github.com/google/clspv/blob/master/docs/OpenCLCOnVulkan.md was more to do with the set of kernels we had to get ported from OpenCL C to Vulkan SPIR-V, not necessarily things that couldn't be implemented.

For example, you could implement the vector of 8/16 elements types by using Vulkan matrices or a struct of vectors, but given the kernels we were targeting did not have this pattern it wasn't worth the engineering effort.

@dneto0

This comment has been minimized.

Show comment
Hide comment
@dneto0

dneto0 Sep 29, 2017

Collaborator

There's a few levels of support we could imagine:

  • level 0: things we can relatively easily map to Vulkan compute shaders today (with variable pointers and 16bit storage), and driven by the samples we based this work on. We've got many bugs to fix before we can call this stable
  • level 1: things that, with more work in the compiler, still could map to Vulkan (with variable pointers and 16bit storage). Example: Adding vectors of 8 or 16 elements.
  • level 2: Like 1, but using more Vulkan features, e.g. shaderFloat64
  • level 3: Like 2, but possibly defining more Vulkan extensions. This would need to be done with cooperation of the Vulkan and maybe SPIR-V working groups.

My current focus is on getting level 0 stabilized. The others would be nice but are far lower priority for me. I'd accept patches for 1 and 2 as long as they don't make stability at level 0 worse or the code quality vastly worse.

(And yes, I should do a better job of communicating this.... )

Collaborator

dneto0 commented Sep 29, 2017

There's a few levels of support we could imagine:

  • level 0: things we can relatively easily map to Vulkan compute shaders today (with variable pointers and 16bit storage), and driven by the samples we based this work on. We've got many bugs to fix before we can call this stable
  • level 1: things that, with more work in the compiler, still could map to Vulkan (with variable pointers and 16bit storage). Example: Adding vectors of 8 or 16 elements.
  • level 2: Like 1, but using more Vulkan features, e.g. shaderFloat64
  • level 3: Like 2, but possibly defining more Vulkan extensions. This would need to be done with cooperation of the Vulkan and maybe SPIR-V working groups.

My current focus is on getting level 0 stabilized. The others would be nice but are far lower priority for me. I'd accept patches for 1 and 2 as long as they don't make stability at level 0 worse or the code quality vastly worse.

(And yes, I should do a better job of communicating this.... )

@tomeuv

This comment has been minimized.

Show comment
Hide comment
@tomeuv

tomeuv Oct 16, 2017

Thanks for the replies. To give some background, we are evaluating using clspv from Mesa/clover for OpenCL. Mesa already contains a translator from Vulkan SPIR-V to NIR, from which drivers can generate native code.

I'm assuming from the above that you wouldn't want for clspv to generate SPIR-V that isn't compliant with Vulkan?

tomeuv commented Oct 16, 2017

Thanks for the replies. To give some background, we are evaluating using clspv from Mesa/clover for OpenCL. Mesa already contains a translator from Vulkan SPIR-V to NIR, from which drivers can generate native code.

I'm assuming from the above that you wouldn't want for clspv to generate SPIR-V that isn't compliant with Vulkan?

@sheredom

This comment has been minimized.

Show comment
Hide comment
@sheredom

sheredom Oct 19, 2017

Contributor

That is an interesting use case! Any contributions that conform to Vulkan SPIR-V would be welcome of course 😄

Contributor

sheredom commented Oct 19, 2017

That is an interesting use case! Any contributions that conform to Vulkan SPIR-V would be welcome of course 😄

@ericberdahl

This comment has been minimized.

Show comment
Hide comment
@ericberdahl

ericberdahl Oct 19, 2017

@tomeuv My impression (as a user of clspv) is that clspv is mostly intended to enable OpenCL C code to be reused in an environment that consumes SPIR-V's shader variant. There exist compilers (even open compilers) that compile OpenCL C into SPIR-V's kernel variant (the form that OpenCL runtimes consume).
So, if you want a compiler that emits SPIR-V and consumes all of OpenCL C, take a look at https://github.com/KhronosGroup/SPIR/tree/spirv-1.0 . If, in contrast, you want a compiler that emits SPIR-V that will run in Vulkan (or runtimes that resemble Vulkan more closely than they resemble OpenCL), keep poking at clspv. The catch with this latter case is that there are structures in OpenCL C that are, at best, difficult to represent in Vulkan.

ericberdahl commented Oct 19, 2017

@tomeuv My impression (as a user of clspv) is that clspv is mostly intended to enable OpenCL C code to be reused in an environment that consumes SPIR-V's shader variant. There exist compilers (even open compilers) that compile OpenCL C into SPIR-V's kernel variant (the form that OpenCL runtimes consume).
So, if you want a compiler that emits SPIR-V and consumes all of OpenCL C, take a look at https://github.com/KhronosGroup/SPIR/tree/spirv-1.0 . If, in contrast, you want a compiler that emits SPIR-V that will run in Vulkan (or runtimes that resemble Vulkan more closely than they resemble OpenCL), keep poking at clspv. The catch with this latter case is that there are structures in OpenCL C that are, at best, difficult to represent in Vulkan.

@dneto0

This comment has been minimized.

Show comment
Hide comment
@dneto0

dneto0 Oct 21, 2017

Collaborator

+1 to what @ericberdahl said.

I'm assuming from the above that you wouldn't want for clspv to generate SPIR-V that isn't compliant with Vulkan?

I'm focused on Vulkan, including how Vulkan evolves over time.

But I'm not going to definitively say "no" to something else. But you would have to be more specific for me to give a more specific answer.

Am I interested in having clspv generate NIR? Not really. I think it's better to go through SPIR-V (for Vulkan) as an intermediate, and that problem is solved as far as I can tell.

Collaborator

dneto0 commented Oct 21, 2017

+1 to what @ericberdahl said.

I'm assuming from the above that you wouldn't want for clspv to generate SPIR-V that isn't compliant with Vulkan?

I'm focused on Vulkan, including how Vulkan evolves over time.

But I'm not going to definitively say "no" to something else. But you would have to be more specific for me to give a more specific answer.

Am I interested in having clspv generate NIR? Not really. I think it's better to go through SPIR-V (for Vulkan) as an intermediate, and that problem is solved as far as I can tell.

@tomeuv

This comment has been minimized.

Show comment
Hide comment
@tomeuv

tomeuv Oct 25, 2017

So, if you want a compiler that emits SPIR-V and consumes all of OpenCL C, take a look at https://github.com/KhronosGroup/SPIR/tree/spirv-1.0 .

Yes, but I have another requirement, which is that this can be properly distributed by third parties from pretty much the start. So Khronos' compiler isn't in the right shape because of being a LLVM fork, even if it's functionally closer to what I need.

Clspv is in the right shape, but there's a big functionality gap. But we can start right away without a long period refactoring LLVM or Khronos' compiler (or most likely, both).

tomeuv commented Oct 25, 2017

So, if you want a compiler that emits SPIR-V and consumes all of OpenCL C, take a look at https://github.com/KhronosGroup/SPIR/tree/spirv-1.0 .

Yes, but I have another requirement, which is that this can be properly distributed by third parties from pretty much the start. So Khronos' compiler isn't in the right shape because of being a LLVM fork, even if it's functionally closer to what I need.

Clspv is in the right shape, but there's a big functionality gap. But we can start right away without a long period refactoring LLVM or Khronos' compiler (or most likely, both).

@tomeuv

This comment has been minimized.

Show comment
Hide comment
@tomeuv

tomeuv Oct 25, 2017

Am I interested in having clspv generate NIR? Not really. I think it's better to go through SPIR-V (for Vulkan) as an intermediate, and that problem is solved as far as I can tell.

We are planning to use the SPIR-V to NIR translator that is already in Mesa. We also want to have SPIR-V in the middle because the same Mesa drivers will want to gain support for OpenCL 2.1 and Vulkan at some point.

tomeuv commented Oct 25, 2017

Am I interested in having clspv generate NIR? Not really. I think it's better to go through SPIR-V (for Vulkan) as an intermediate, and that problem is solved as far as I can tell.

We are planning to use the SPIR-V to NIR translator that is already in Mesa. We also want to have SPIR-V in the middle because the same Mesa drivers will want to gain support for OpenCL 2.1 and Vulkan at some point.

@tomeuv

This comment has been minimized.

Show comment
Hide comment
@tomeuv

tomeuv Jan 9, 2018

Thanks for the feedback. Currently I'm planning on forking Khronos' converter, as explained in http://lists.llvm.org/pipermail/llvm-dev/2018-January/120291.html

tomeuv commented Jan 9, 2018

Thanks for the feedback. Currently I'm planning on forking Khronos' converter, as explained in http://lists.llvm.org/pipermail/llvm-dev/2018-January/120291.html

@tomeuv tomeuv closed this Jan 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment