Clean up OpenCL compilation into SPIR-V. #4375
Conversation
There was a problem hiding this comment.
Hi! Thanks for this.
Unfortunately as this changes compiler ids, we can't merge as-is, as old links with these compilers would break.
To fix this, either leave the old ids (Which maybe not the best idea given that removing arm references is the point of this PR), or for every changed compiler id, add a prop like:
compiler.<newid>.alias=<oldid> so that old links will be able to still find the compiler
This change removes any references to Arm in SPIR-V compiler for OpenCL as there isn't anything specific to Arm in the compilation process. It also avoids using Arm specific workaround because SPIR-V compilation in clang doesn't suffer from the same limitation. This change also makes explicit the fact that compilation uses llvm-spirv as clang now has internal SPIR-V support too which should be added here once it matures.
AnastasiaStulova
force-pushed
the
opencl_spirv
branch
from
c0b682c
to
746fd13
Compare
Thanks for the suggestion. I have added alias to all the compilers being renamed. |
| group.armcpp4oclclang32spir.compilerType=spirv | ||
| group.armcpp4oclclang32spir.supportsExecute=false | ||
| group.armcpp4oclclang32spir.instructionSet=arm32 | ||
| # The -Dkernel= -D__kernel= workaround is required to prevent the Clang crash reported in https://llvm.org/PR50841 |
There was a problem hiding this comment.
As far as I can see, your PR fixing this hasn't yet been merged, right? (LLVM's issue tracker is a bit hard to read, so sorry if I missed something) Do we not need the workaround anymore?
There was a problem hiding this comment.
@AnastasiaStulova thanks again for your contribution: we'd love to merge. Can you answer @RubenRBS 's question so we know where we are on this?
There was a problem hiding this comment.
Sorry for delay, this clang bug is only for compiling to Arm targets. When we compile for SPIR/SPIR-V: -triple spir, -triple spir64, -triple spirv32 or -triple spirv64 no workaround is needed as clang just compiles correctly.
|
Thanks: I'm going to merge this so we don't get too stale. Sorry for the delay. |
This change removes any references to Arm in SPIR-V compiler for OpenCL as there isn't anything specific to Arm in the compilation process. It also avoids using Arm specific workaround because SPIR-V compilation in clang doesn't suffer from the same limitation.
This change also makes explicit the fact that compilation uses llvm-spirv as clang now has internal SPIR-V support too which should be added here once it matures.