[Feature Request] Make DXC emit VulkanMemoryModel instead of Logical GLSL450

[kpentaris]

Is your feature request related to a problem? Please describe. We would like to take advantage of the Vulkan Memory Model, for consistency and sanity reasons.

Describe the solution you'd like A compilation flag that would enable the Vulkan memory model

Describe alternatives you've considered Use GLSL450 but we'd like to be able to reason about the memory ordering of our operations. There is also the flag -0config=--upgrade-memory-model but this would require writing operations in glsl450 and rely on spirv-opt to upgrade them, whereas we'd like to use the proper VMM opcodes which are not necessarily expressible in glsl450.

Additional context N/A

[sudonatalie]

Thanks for submitting this issue! This feature isn't likely to be prioritized in the near future, however, if you are able to provide any examples of HLSL that is sub-optimally expressed when going through OpMemoryModel Logical GLSL450->--upgrade-memory-model that would be helpful to better understand your use case.

[devshgraphicsprogramming]

The problem is that we can't use VMM (like atomics, OpLoad and OpStore with acquire/release operands) while DXC still emits GLSL450 for all the rest of the code

[devshgraphicsprogramming]

The problem is that we can't use VMM (like atomics, OpLoad and OpStore with acquire/release operands) while DXC still emits GLSL450 for all the rest of the code

@sudonatalie I have an example! (with BDA and prop 0011 syntax) https://godbolt.org/z/a9sx4ss68

and still we can't use acquire/release etc on atomics

fatal error: generated SPIR-V is invalid: VulkanMemoryModelKHR capability must only be specified if the VulkanKHR memory model is used.
  OpMemoryModel Logical GLSL450

[sudonatalie]

Thanks for the additional info @devshgraphicsprogramming. We'll keep this on our radar but it's unlikely to be a near future priority.

[devshgraphicsprogramming]

Thanks for the additional info @devshgraphicsprogramming. We'll keep this on our radar but it's unlikely to be a near future priority.

Actually turned out I didn't need this to declare my OpLoad for BDA was missing a [[vk::ext_literal]] on the intrinsic declaration