KhronosGroup / OpenCL-Docs

OpenCL API, OpenCL C, Extensions, SPIR-V Environment Specs, Ref page, and C++ for OpenCL doc sources.
Other
358 stars 112 forks source link

SPIR-V: Validation rules for memory semantics storage classes? #91

Open bashbaug opened 5 years ago

bashbaug commented 5 years ago

Question from https://github.com/KhronosGroup/OpenCL-Docs/pull/89#issuecomment-503557202:

Memory Semantics is used to specify both memory-order constraints and the storage classes they apply to.

Make it explicit that only the memory-order constraint bits are restricted. The storage class can vary (WorkgroupMemory or CrossWorkgroupMemory).

do we want 1.2 validation rules for the storage classes? Can we define them more generally?

bashbaug commented 5 years ago

do we want 1.2 validation rules for the storage classes? Can we define them more generally?

Interesting question. Of the storage classes in Memory Semantics, four apply to SPIR-V kernels: SubgroupMemory, WorkgroupMemory, CrossWorkgroupMemory, and ImageMemory. Of these, WorkgroupMemory (equivalent to OpenCL local memory) and CrossWorkgroupMemory (equivalent to OpenCL global memory) should clearly be valid in all environments, so I think the question is really about SubgroupMemory and ImageMemory.

ImageMemory is needed for OpenCL 2.0 for read-write images and CLK_IMAGE_MEM_FENCE. I don't think it'd hurt anything in earlier environments, but it's not very useful, either. I don't think SubgroupMemory has meaning in any OpenCL environment.

If we decide that ImageMemory and/or SubgroupMemory storage classes don't have meaning in at least some OpenCL envirnoments, should they be ignored, or should they make a module invalid?

Note that the Vulkan environment says:

  • Memory semantics must obey the following rules:
    • ...
    • SubgroupMemory, CrossWorkgroupMemory, and AtomicCounterMemory are ignored.
kpet commented 5 years ago

There is also the question of what combinations should be valid which you touched on in #88.

should they be ignored, or should they make a module invalid?

I think my preference would be that they make the module invalid. If they are ignored we run the risk that producers generate them without consequences today but the same module could start misbehaving with future implementations if these storage classes start having an associated behaviour (although any such behaviour would probably require a new capability or new SPIR-V version).