KhronosGroup / KhronosGroup/SPIRV-LLVM-Translator
Wrong atomic instructions translation from SPIRV to LLVM [OpenCL 1.2]
- Dominant language
- LLVM
- Stars
- 625
- Forks
- 279
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 34
Description
Currently all spirv atomic instructions are translated into OpenCL 2.0 llvm builtins, despite of the value specified in `-spirv-ocl-builtins-version`.
Let's consider that OpenCL 1.2 kernel is translated from LLVM to SPIRV and then back to LLVM.
LLVM: `atomic_add(global or local ptr)` --> SPIRV: `OpAtomicIAdd ` --> LLVM: `fetch_add_explicit(global or local ptr)`
The first issue here is that translator produces OpenCL 2.0 version of function and the second (most important) is that this builtin is not even OpenCL-consistent, because fetch_add_explicit takes ptr with generic address space, not global or local.
I've resolved this issue for almost all atomic instructons (https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/263) except OpAtomicLoad, OpAtomicStore, OpAtomicFlagTestAndSet and OpAtomicFlagClear.
How should we translate f.eg. OpAtomicLoad for OpenCL 1.2? There is no counterpart instruction for this one in OpenCL 1.2. Should we issue an assert if SPIRV module contains OpAtomicLoad and `CL1.2` is specified by user? I think it's incorrect, because OpAtomicLoad is not specified as not supported in OpenCL 1.2 validation rules (https://www.khronos.org/registry/OpenCL/specs/2.2/html/OpenCL_Env.html#_validation_rules_5). Maybe we should somehow emulate this instruction with regular OpenCL 1.2 load instruction (not atomic).
I've opened this issue to discuss the best, proper way of translating those four instructions for OpenCL 1.2. Would be great to hear what's your opinion about that.
Contributor guide
Research direction
Start by reviewing the existing atomic-instruction changes in PR #263 and the handling selected by `-spirv-ocl-builtins-version`. Focus on `OpAtomicLoad`, `OpAtomicStore`, `OpAtomicFlagTestAndSet`, and `OpAtomicFlagClear` for OpenCL 1.2, and consult the linked OpenCL validation rules. Done means a decided, consistent translation or rejection policy for all four instructions.
Written by the indexing model from the issue text.
Assessment
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100