clvk icon indicating copy to clipboard operation
clvk copied to clipboard

Support CTS compiler tests involving link

Open rjodinchr opened this issue 3 years ago • 2 comments

Most of the CTS compiler tests clvk does not support involve the clLinkProgram function.

At the moment, clvk implementation is based on the spirv-tools, using SPIRV as the intermediate language to link multiple programs. It feels quite complicated to have it working as the OpenCL intends it to.

Instead of using SPIRV as the intermediate language, I propose to just simply use the LLVM IR. It means that we would need to be able to link LLVM IR. We could do that using llvm tools, or just have it integrated inside clspv.

I think in the end, it would be a better and easier to maintain solution. clspv often needs to have all the information about the kernel (and all the subfunctions) to be able to generate correct SPIRV code. That way, we would ensure that clspv always has that information.

  • [ ] clCompileProgram needs to produce LLVM IR
  • [ ] clLinkProgram needs to link LLVM IR
  • [ ] CL_PROGRAM_BINARIES could either be a LLVM IR program (which would need to be compile with clspv to be executed), or a SPIRV program that is "ready" to be executed.

rjodinchr avatar Jul 28 '22 08:07 rjodinchr

This is a suggested alternative way:

  • Add an optional flag to clspv that says it's safe to add the Linkage capability to the SPIR-V and then add function declarations with Internal/External linkage etc as expected
  • clvk consumes the SPIR-V and passes it to the spv linker which will do proper linking
  • clvk then verifies that there are no dangling links, then goes through the module manually to strip the OpCapability Linkage and any other related decorations
  • when enqueued the binary is sent to the Vulkan driver with no traces of linkage to be found so it's considered a valid binary

I thought of adding it for maybe if LLVM IR method had any unexpected problems, or undesirable for some reason, or if you think this could be a better choice.

omarahmed1111 avatar Aug 12 '22 13:08 omarahmed1111

Yes, I have also considered this solution. I think the actual code is closer to this solution than the one I have proposed above.

The issue is that clspv often needs to have all the information to know which function to inline in order to be able to generate the correct spirv code. I think the solution you suggested might work for the CTS, but those are really simple kernels. I am not sure it will support real applications using this feature.

rjodinchr avatar Aug 12 '22 13:08 rjodinchr