Skip to content

spirv-val: Update memory semantics rules to match the specification#6096

Merged
alan-baker merged 2 commits intoKhronosGroup:mainfrom
natgavrilenko:validation-vulkan-semantics
Aug 7, 2025
Merged

spirv-val: Update memory semantics rules to match the specification#6096
alan-baker merged 2 commits intoKhronosGroup:mainfrom
natgavrilenko:validation-vulkan-semantics

Conversation

@natgavrilenko
Copy link
Copy Markdown
Contributor

@natgavrilenko natgavrilenko commented Apr 17, 2025 •
edited
Loading

This commit updates the validation of memory semantics to match the formal Vulkan memory model and the Vulkan specification:

  • Require non-empty storage class semantics for non-relaxed memory orders.
  • Require non-relaxed memory orders for non-empty storage class semantics.
  • Validate AtomicCompareExchange Unequal semantics with respect to its Equal semantics.
  • Unify Vulkan error messages and validation rules.

Closes #6216.

Summary of changes:

  1. OpAtomicStore with Acquire or AcqRel memory order
  • Before: Forbidden only if the target environment is Vulkan.
  • After: Always forbidden.
  1. OpAtomicLoad with Release or AcqRel memory order
  • Before: Forbidden only if the target environment is Vulkan.
  • After: Always forbidden.
  1. OpMemoryBarrier with Relaxed memory order
  • Before: Forbidden only if the target environment is Vulkan.
  • After: Forbidden if the target environment or memory model is Vulkan.

Note: According to the OpenCL specification, a relaxed mem_fence has no effect but is not explicitly forbidden.

  1. SequentiallyConsistent memory order
  • Before: Forbidden only if the memory model is Vulkan.
  • After: Forbidden if the target environment or memory model is Vulkan.
  1. Non-relaxed memory order requires non-empty storage class semantics, and vice versa
  • Before: Enforced for OpMemoryBarrier and OpControlBarrier if the target environment is Vulkan.
  • After: Enforced for all instructions if the target environment or memory model is Vulkan.

Note: Similar to the original validation rule for the barriers, the new rule considers only UniformMemory, WorkgroupMemory, ImageMemory, and OutputMemory storage class semantics. According to the Vulkan specification, the other storage class semantics (SubgroupMemory, AtomicCounterMemory, and CrossWorkgroupMemory) are not supported by Vulkan.

  1. Avvis semantics requires storage class semantics
  • Before: All storage class semantics bits were considered.
  • After: Only Vulkan-supported storage class semantics bits (UniformMemory, WorkgroupMemory, ImageMemory, and OutputMemory) are considered.
  1. AtomicCompareExchange Unequal memory order must not be stronger than Equal memory order
  • Before: Never enforced.
  • After: Always enforced.
  1. AtomicCompareExchange Unequal storage class and avvis semantics must be included in Equal
  • Before: Never enforced.
  • After: Enforced if the target environment or memory model is Vulkan.

@natgavrilenko natgavrilenko marked this pull request as draft April 17, 2025 13:25
@natgavrilenko natgavrilenko force-pushed the validation-vulkan-semantics branch from 93024d9 to a9ebc33 Compare April 30, 2025 11:24
@natgavrilenko natgavrilenko force-pushed the validation-vulkan-semantics branch from a9ebc33 to bfa6d1a Compare July 14, 2025 16:20
@natgavrilenko natgavrilenko changed the title DRAFT: Updated validation of memory semantics spirv-val: Update memory semantics rules to match the specification Jul 14, 2025
@natgavrilenko natgavrilenko marked this pull request as ready for review July 14, 2025 16:22
@natgavrilenko natgavrilenko force-pushed the validation-vulkan-semantics branch from bfa6d1a to 6b7df25 Compare July 15, 2025 11:00
@natgavrilenko
Copy link
Copy Markdown
Contributor Author

natgavrilenko commented Jul 22, 2025

I see the glslang tests are failing due to the check for relaxed memory order with non-zero storage class semantics.

@alan-baker how would you prefer to proceed?

  1. Temporarily disable this check and re-enable it in a follow-up PR after updating glslang? I can either try fixing it myself or create an issue for them.
  2. Postpone this PR and update glslang first.
  3. Something else?

@alan-baker
Copy link
Copy Markdown
Contributor

alan-baker commented Jul 22, 2025

I see the glslang tests are failing due to the check for relaxed memory order with non-zero storage class semantics.

@alan-baker how would you prefer to proceed?

  1. Temporarily disable this check and re-enable it in a follow-up PR after updating glslang? I can either try fixing it myself or create an issue for them.
  2. Postpone this PR and update glslang first.
  3. Something else?

If the fix for glslang is quick, we can hold off on this PR. Otherwise, we merge this and then there are likely two changes for glslang:

  1. update spirv-tools deps and test expectations
  2. fix glslang codegen

So if glslang could be fixed in a week or two that might be preferable.

@natgavrilenko
Copy link
Copy Markdown
Contributor Author

natgavrilenko commented Jul 22, 2025

If the fix for glslang is quick, we can hold off on this PR. Otherwise, we merge this and then there are likely two changes for glslang:

  1. update spirv-tools deps and test expectations
  2. fix glslang codegen

So if glslang could be fixed in a week or two that might be preferable.

Okay, I will take a closer look at the glslang problem and let you know the status in a few days.

natgavrilenko added a commit to natgavrilenko/glslang that referenced this pull request Jul 27, 2025
@natgavrilenko
Fix memory semantics validation rules according to the spec
b702565 
This commit updates memory semantics validation rules to match
Vulkan specification update
KhronosGroup/Vulkan-Docs#2528
and the spirv-tools update
KhronosGroup/SPIRV-Tools#6096

Signed-off-by: Natalia Gavrilenko <natalia.gavrilenko@huawei.com>
natgavrilenko added a commit to natgavrilenko/glslang that referenced this pull request Jul 27, 2025
@natgavrilenko
Fix memory semantics validation rules according to the spec
99cec4a 
This commit updates memory semantics validation rules to match
Vulkan specification update
KhronosGroup/Vulkan-Docs#2528
and spirv-tools update
KhronosGroup/SPIRV-Tools#6096

Signed-off-by: Natalia Gavrilenko <natalia.gavrilenko@huawei.com>
@natgavrilenko natgavrilenko mentioned this pull request Jul 27, 2025
Merged
@natgavrilenko natgavrilenko force-pushed the validation-vulkan-semantics branch 2 times, most recently from ccc3057 to 6246406 Compare July 29, 2025 12:34
dnovillo pushed a commit to KhronosGroup/glslang that referenced this pull request Jul 31, 2025
@natgavrilenko
Fix memory semantics validation rules according to the spec (#4008)
579d34d 
This commit updates memory semantics validation rules to match
Vulkan specification update
KhronosGroup/Vulkan-Docs#2528
and spirv-tools update
KhronosGroup/SPIRV-Tools#6096

Signed-off-by: Natalia Gavrilenko <natalia.gavrilenko@huawei.com>
@alan-baker
Copy link
Copy Markdown
Contributor

alan-baker commented Aug 6, 2025

@natgavrilenko could you please rebase this PR on top of main?

@natgavrilenko
spirv-val: Update memory semantics rules to match the specification
143597f 
Updated validation of memory semantics to match the formal Vulkan memory model and the Vulkan specification:
- Require non-empty storage class semantics for non-relaxed memory orders
- Require non-relaxed memory orders for non-empty storage class semantics
- Validate AtomicCompareExchange Unequal semantics with respect to its Equal semantics
- Unify Vulkan error messages and validation rules

Signed-off-by: Natalia Gavrilenko <natalia.gavrilenko@huawei.com>
@natgavrilenko
Removed duplicated tests
ad25402
@natgavrilenko natgavrilenko force-pushed the validation-vulkan-semantics branch from 6246406 to ad25402 Compare August 7, 2025 10:55
@natgavrilenko
Copy link
Copy Markdown
Contributor Author

natgavrilenko commented Aug 7, 2025

could you please rebase this PR on top of main?

@alan-baker Done

@alan-baker alan-baker enabled auto-merge (squash) August 7, 2025 17:40
@alan-baker alan-baker merged commit 5e7108e into KhronosGroup:main Aug 7, 2025
25 of 26 checks passed
@natgavrilenko natgavrilenko deleted the validation-vulkan-semantics branch November 7, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alan-baker alan-baker alan-baker approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

spirv-val: Add (or label) new MemorySemantics VUs

3 participants

@natgavrilenko @alan-baker @kokoro-team