KhronosGroup / KhronosGroup/SPIRV-LLVM-Translator
translating OpenCL C barrier memory semantics to SPIR-V
- Dominant language
- LLVM
- Stars
- 625
- Forks
- 279
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 34
Description
I noticed a couple of potential issues with the current translation from OpenCL C barriers to SPIR-V while adding support for split barriers recently (https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/1424). It's possible these are issues with the OpenCL SPIR-V environment spec and not the SPIR-V LLVM Translator, or that clarifications are needed in both places.
* The SPIR-V LLVM Translator currently turns a call to the OpenCL C barrier function to a SPIR-V OpControlBarrier with SequentiallyConsistent memory semantics. Specifically, it turns this:
```c
barrier(CLK_LOCAL_MEM_FENCE);
```
into this:
```
; Op0 = Scope for Execution = 2 = Workgroup
; Op1 = Scope for Memory = 2 = Workgroup
; Op2 = Memory Semantics = 272 = 0x110 = SequentiallyConsistent (0x10) | WorkgroupMemory (0x100)
OpControlBarrier %uint_2 %uint_2 %uint_272
```
This is consistent with the OpenCL SPIR-V environment spec, but is this intended? Specifically, should this be changed to use AcquireRelease memory semantics instead?
Note that the mandated OpenCL 3.0 minimum device capabilities for [CL_DEVICE_ATOMIC_FENCE_CAPABILITIES](https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_API.html#CL_DEVICE_ATOMIC_FENCE_CAPABILITIES) includes ACQ_REL but not SEQ_CST, so arguably an OpenCL 3.0 implementation supporting the minimum device capabilities wouldn't be able to consume the OpControlBarrier as translated today.
* The SPIR-V LLVM Translator currently has a special case for an OpenCL C barrier function with no memory space and turns this into a SPIR-V OpControlBarrier with Relaxed memory semantics. Specifically, it turns this:
```c
barrier(0);
```
into this:
```
; Op2 = Memory Semantics = 0 = None (Relaxed)
OpControlBarrier %uint_2 %uint_2 %uint_0
```
Strictly speaking, an OpControlBarrier with Relaxed memory semantics is not valid according to the OpenCL SPIR-V environment spec. Should this case be added? Or should we remove this special case and generate a SequentiallyConsistent or AcquireRelease barrier instead, like usual? Note that the memory semantics does not include any memory spaces, so a SPIR-V consumer would still be able to optimize this case, if desired (assuming any real-world code actually does this).
Contributor guide
Research direction
Start by tracing how OpenCL C barrier calls become OpControlBarrier instructions in the SPIR-V LLVM Translator, then compare both memory-semantics cases with the OpenCL SPIR-V environment specification and OpenCL 3.0 fence capabilities. Done requires resolving whether the translator or specification needs a change, including the expected semantics for barrier(0).
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
- Needs clarification
- Newbie friendliness
- 32/100