Skip to content
This repository has been archived by the owner on Oct 9, 2019. It is now read-only.

OpenCL file compiled by Khronos OpenCL Clang compiler+SPIRV-LLVM using cl_intel_subgroups+cl_intel_subgroups_short omits needed OpExtension "SPV_INTEL_subgroups" and also Vector16 capability in generated SPIR-V #228

Open
oscarbg opened this issue Feb 20, 2018 · 10 comments
Labels
migrate To be moved to the new repo

Comments

@oscarbg
Copy link

oscarbg commented Feb 20, 2018

Hi,
@bashbaug @AlexeySotkin @yxsamliu
the issue in fact is detailed here:
KhronosGroup/SPIRV-Tools#1327
can you please take a detailed look.. you have the OpenCL cl file, the compiler arguments and the resulting spirv file and output from spirv-val..

@bashbaug
Copy link
Contributor

Thanks for the report. I'll take a look.

@oscarbg
Copy link
Author

oscarbg commented Feb 20, 2018

let me ask some somewhat related questions:
don't tested yet but it's that SPIR-V shaders usable on any public Intel OpenCL driver either on Windows or Linux platform..
also how is related to new Intel OpenCL Neo driver (https://github.com/intel/compute-runtime)..
it supports SPV intel subgroups already?
just opened an issue just aksing for info here:
intel/compute-runtime#13
perhaps can contribute there..
and finally one last question:
I'm having trouble compiling the OpenCL file attached below (Opt_MatrixMultiplication.cl) to SPIR-V of this sample obtained from Intel site using the same Khronos OpenCL Clang compiler+SPIRV-LLVM I compiled from master that has "SPV_INTEL_subgroups":
GEMM_sample_Jun2017.zip

it uses cl_intel_subgroups but I see in code also something like:
#ifdef cl_intel_simd_operations_placeholder
#define cl_intel_subgroups
#define sub_group_broadcast intel_simd_shuffle
#endif

so is this OpenCL file compilable yet to SPIR-V..
I say because it seems the fastest GEMM code avaiable for Intel HD Graphics right now and also with some very optimized f16 matmal code..
would be happy if you can take a look and comment..

@bashbaug
Copy link
Contributor

I had a look and here's what I found.

  1. The Vector16 capability is the easy one. There was a change recently so the Vector16 capability was only added if vectors of 16 elements are used, which matches the name of the capability, but not its semantics. The change needs to be reverted so the capability is added even if only vectors of 8 elements are used. I'll create a pull request to fix this shortly.

  2. The extension one is a bit more complicated. In short, the extension handling in this SPIR-V generator is not quite correct (it's not quite correct in the published OpenCL Environment Spec, either - we're already fixing this). I'm looking at options to fix this, but it's going to take a bit more investigation.

I'll address the SPIR-V consumption questions in the issue you filed in the GPU Compute Runtime repo (intel/compute-runtime#13), if that's OK, to keep the two issues separate.

Thanks again!

@bader
Copy link
Contributor

bader commented Feb 25, 2018

The Vector16 capability is the easy one. There was a change recently so the Vector16 capability was only added if vectors of 16 elements are used, which matches the name of the capability, but not its semantics. The change needs to be reverted so the capability is added even if only vectors of 8 elements are used. I'll create a pull request to fix this shortly.

Vector16 is unfortunate name for the capability enabling 8-element vectors. Can we fix this inconsistency in SPIR-V specification?

@bashbaug
Copy link
Contributor

I created pull request #229 to address the Vector16 capability.

I have a hacky fix for the extension handling that I've checked into a branch in my fork. The basic idea is to look at the capabilities the SPIR-V module requires, and to emit the appropriate SPIR-V OpExtensions as needed. It needs some work before I can create a pull request, but if you're curious feel free to have a look:

https://github.com/bashbaug/SPIRV-LLVM/tree/spirv-extensions

@oscarbg
Copy link
Author

oscarbg commented Feb 28, 2018

Hi,
thanks for your support.. will give a try to both fixes.. just a question, just noticed a new spirv 6.0 branch by @AlexeySotkin which I also want to test.. do you know if has same problems? i.e. requires similar fixes too?

@AlexeySotkin
Copy link
Contributor

Hi,
Yes, the new branch requires this fix as well.

@oscarbg
Copy link
Author

oscarbg commented Mar 2, 2018

Hi @AlexeySotkin,
thanks.. seems this branch is good also for POCL SPIR-V support in new upcoming POCL 1.1 release so thanks for your work.. (confirmed by @franz )
just for info, don't know if it makes much sense to ask: can we expect also some new Clang 6.0 SPIR supporting branch? i.e. updating previous 3.6 one: https://github.com/KhronosGroup/SPIR/tree/spirv-1.0 branch with updated

@bader
Copy link
Contributor

bader commented Mar 2, 2018

@oscarbg, thanks for looking into this.
This is not related to the bug reported here, but just to let know, spirv-6.0 was taken from the LLVM trunk, not from LLVM 6.0 release branch, so it might be not a release quality.
Another important point is that this branch is created to facilitate integration of SPIR-V translation library with llvm.org repository. We are going to do some code refactoring which is under discussion with LLVM community: http://lists.llvm.org/pipermail/llvm-dev/2018-February/thread.html#121317

Regarding integration with other tools we hope that new repository will support existing Khronos OpenCL C and OpenCL C++ compilers, but instead of supporting Clang forks on GitHub, we plan contributing OpenCL C/OpenCL C++ support to llvm.org repository (http://lists.llvm.org/pipermail/cfe-dev/2018-February/056972.html). So eventually we are going to enable OpenCL C and OpenCL C++ support in Clang with an option to produce SPIR-V form.
OpenCL C++ support might take some time, but it should be quite easy get OpenCL C to SPIR-V compiler.
Until then I would avoid using spirv-60 branch for any serious projects as there might be significant changes.

@oscarbg
Copy link
Author

oscarbg commented Mar 5, 2018

really thanks @bader for the general overview of the current state of LLVM/Clang SPIRV support and how things are evolving..
the future looks great if OpenCL C++ and SPIR-V output are merged into mainline Clang/LLVM project..

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
migrate To be moved to the new repo
Projects
None yet
Development

No branches or pull requests

4 participants