questionable Memory Semantics for OpenCL C atomic functions

[bashbaug]

For Khronos folks, this is partially discussed in internal issue 152.

The "Memory Semantics" that are generated for the OpenCL C 2.0 atomic functions are debatably incorrect. Consider this OpenCL C kernel, which performs an atomic store to an atomic variable in the global address space, performing a release operation:

kernel void test(global atomic_int* gptr, local atomic_int* lptr) {
    atomic_store_explicit(gptr, 2, memory_order_release);
}

The SPIR-V atomic instruction that is generated for this kernel is something like:

OpAtomicStore %13 %uint_1 %uint_4 %uint_2

Here are the operands for this instruction:

1. %13 is the pointer operand, from previous instructions.
2. %uint_1 is the Scope for Memory, and represents Device scope. This makes sense, since the default memory scope is memory_scope_device: "The functions that do not have memory_scope argument have the same semantics as the corresponding functions with the memory_scope argument set to memory_scope_device."
3. %uint_4 is the Memory Semantics. This represents the Release memory order, which is good, but strangely there are no affected memory spaces.
4. %uint_2 is the value that is stored, and is fine.

Should the SPIR-V LLVM Translator include affected memory spaces in the generated atomic instruction? If so, how should it determine which memory spaces are affected? If not, what should it mean to have an atomic memory order, but no affected memory spaces?

[MrSidims]

I agree with the proposal you shared internally - use pointer's address space to determine, which memory semantic to use. In case if address space is generic and it's not possible to infer it at the point of the translation - we should use conservative mask CrossWorkgroupMemory | WorkgroupMemory | SubgroupMemory (whatever subgroup memory means).

[bashbaug]

Yeah, I haven't thought of a better resolution, and I like how this avoids the need to define some sort of default behavior when no memory spaces are included in the memory semantics.

[Aside, implementations must be doing something in this case though, since it's what the SPIR-V LLVM Translator is currently generating...]

Longer-term, I wonder if SPIR-V modules with an Acquire, Release, AcquireRelease, or SequentiallyConsistent memory order and no affected memory spaces should be considered invalid, since this is almost certainly a bug.

[MrSidims]

Created #3375