From 3d710e7c4b9792f7c9d2aaae91536c4698db153b Mon Sep 17 00:00:00 2001 From: Markus Siglreithmaier Date: Thu, 22 Apr 2021 11:17:02 +0200 Subject: [PATCH] Fix semantics type for control barriers to support multiple flags (#598) * Fix semantics type for control barriers to support multiple flags * spirv-std: convert memory semantics to bitflag * Update crates/spirv-std/src/arch/barrier.rs Co-authored-by: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com> Co-authored-by: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com> --- crates/spirv-std/src/arch/barrier.rs | 10 +-- crates/spirv-std/src/memory.rs | 110 +++++++++++++-------------- tests/ui/arch/control_barrier.rs | 2 +- tests/ui/arch/memory_barrier.rs | 6 +- 4 files changed, 66 insertions(+), 62 deletions(-) diff --git a/crates/spirv-std/src/arch/barrier.rs b/crates/spirv-std/src/arch/barrier.rs index 56ac3c9c91..df255fab96 100644 --- a/crates/spirv-std/src/arch/barrier.rs +++ b/crates/spirv-std/src/arch/barrier.rs @@ -11,12 +11,12 @@ use crate::memory::{Scope, Semantics}; /// this instruction. When Execution is Subgroup or Invocation, the behavior of /// this instruction in non-uniform control flow is defined by the client API. /// -/// If [`Semantics`] is not [`Semantics::None`], this instruction also serves as +/// If [`Semantics`] is not [`Semantics::NONE`], this instruction also serves as /// an [`memory_barrier`] function call, and also performs and adheres to the /// description and semantics of an [`memory_barrier`] function with the same /// `MEMORY` and `SEMANTICS` operands. This allows atomically specifying both a /// control barrier and a memory barrier (that is, without needing two -/// instructions). If [`Semantics`] is [`Semantics::None`], `MEMORY` is ignored. +/// instructions). If [`Semantics`] is [`Semantics::NONE`], `MEMORY` is ignored. /// /// Before SPIRV-V version 1.3, it is only valid to use this instruction with /// `TessellationControl`, `GLCompute`, or `Kernel` execution models. There is @@ -43,7 +43,7 @@ pub unsafe fn control_barrier< "OpControlBarrier %execution %memory %semantics", execution = const EXECUTION as u8, memory = const MEMORY as u8, - semantics = const SEMANTICS as u8, + semantics = const SEMANTICS.bits(), } } @@ -66,13 +66,13 @@ pub unsafe fn control_barrier< #[doc(alias = "OpMemoryBarrier")] #[inline] // FIXME(eddyb) use a `bitflags!` `Semantics` for `SEMANTICS`. -pub unsafe fn memory_barrier() { +pub unsafe fn memory_barrier() { asm! { "%u32 = OpTypeInt 32 0", "%memory = OpConstant %u32 {memory}", "%semantics = OpConstant %u32 {semantics}", "OpMemoryBarrier %memory %semantics", memory = const MEMORY as u8, - semantics = const SEMANTICS, + semantics = const SEMANTICS.bits(), } } diff --git a/crates/spirv-std/src/memory.rs b/crates/spirv-std/src/memory.rs index 6bfc2e184b..37e49b8904 100644 --- a/crates/spirv-std/src/memory.rs +++ b/crates/spirv-std/src/memory.rs @@ -21,73 +21,73 @@ pub enum Scope { QueueFamily = 5, } -// FIXME(eddyb) use `bitflags!` for this. -#[derive(Debug, PartialEq, Eq)] -pub enum Semantics { - /// No memory semantics. - None = 0, +bitflags::bitflags! { + pub struct Semantics: u32 { + /// No memory semantics. + const NONE = 0; - /// On an atomic instruction, orders memory operations provided in program - /// order after this atomic instruction against this atomic instruction. On - /// a barrier, orders memory operations provided in program order after this - /// barrier against atomic instructions before this barrier. - Acquire = 0x2, + /// On an atomic instruction, orders memory operations provided in program + /// order after this atomic instruction against this atomic instruction. On + /// a barrier, orders memory operations provided in program order after this + /// barrier against atomic instructions before this barrier. + const ACQUIRE = 0x2; - /// On an atomic instruction, orders memory operations provided in program - /// order before this atomic instruction against this atomic instruction. On - /// a barrier, orders memory operations provided in program order before - /// this barrier against atomic instructions after this barrier. - Release = 0x4, + /// On an atomic instruction, orders memory operations provided in program + /// order before this atomic instruction against this atomic instruction. On + /// a barrier, orders memory operations provided in program order before + /// this barrier against atomic instructions after this barrier. + const RELEASE = 0x4; - /// Has the properties of both [`Self::Acquire`] and [`Self::Release`] semantics. It - /// is used for read-modify-write operations. - AcquireRelease = 0x8, + /// Has the properties of both [`Self::ACQUIRE`] and [`Self::RELEASE`] semantics. It + /// is used for read-modify-write operations. + const ACQUIRE_RELEASE = 0x8; - /// All observers see this memory access in the same order with respect to - /// other sequentially-consistent memory accesses from this invocation. - /// If the declared memory model is `vulkan`, `SequentiallyConsistent` must - /// not be used. - SequentiallyConsistent = 0x10, + /// All observers see this memory access in the same order with respect to + /// other sequentially-consistent memory accesses from this invocation. + /// If the declared memory model is `vulkan`, `SEQUENTIALLY_CONST` must + /// not be used. + const SEQUENTIALLY_CONST = 0x10; - /// Apply the memory-ordering constraints to - /// [`crate::storage_class::StorageBuffer`], - /// [`crate::storage_class::PhysicalStorageBuffer`], or - /// [`crate::storage_class::Uniform`] Storage Class memory. - UniformMemory = 0x40, + /// Apply the memory-ordering constraints to + /// [`crate::storage_class::StorageBuffer`], + /// [`crate::storage_class::PhysicalStorageBuffer`], or + /// [`crate::storage_class::Uniform`] Storage Class memory. + const UNIFORM_MEMORY = 0x40; - /// Apply the memory-ordering constraints to subgroup memory. - SubgroupMemory = 0x80, + /// Apply the memory-ordering constraints to subgroup memory. + const SUBGROUP_MEMORY = 0x80; - /// Apply the memory-ordering constraints to - /// [`crate::storage_class::Workgroup`] Storage Class memory. - WorkgroupMemory = 0x100, + /// Apply the memory-ordering constraints to + /// [`crate::storage_class::Workgroup`] Storage Class memory. + const WORKGROUP_MEMORY = 0x100; - /// Apply the memory-ordering constraints to - /// [`crate::storage_class::CrossWorkgroup`] Storage Class memory. - CrossWorkgroupMemory = 0x200, + /// Apply the memory-ordering constraints to + /// [`crate::storage_class::CrossWorkgroup`] Storage Class memory. + const CROSS_WORKGROUP_MEMORY = 0x200; - /// Apply the memory-ordering constraints to - /// [`crate::storage_class::AtomicCounter`] Storage Class memory. - AtomicCounterMemory = 0x400, + /// Apply the memory-ordering constraints to + /// [`crate::storage_class::AtomicCounter`] Storage Class memory. + const ATOMIC_COUNTER_MEMORY = 0x400; - /// Apply the memory-ordering constraints to image contents (types declared - /// by `OpTypeImage`), or to accesses done through pointers to the - /// [`crate::storage_class::Image`] Storage Class. - ImageMemory = 0x800, + /// Apply the memory-ordering constraints to image contents (types declared + /// by `OpTypeImage`), or to accesses done through pointers to the + /// [`crate::storage_class::Image`] Storage Class. + const IMAGE_MEMORY = 0x800; - /// Apply the memory-ordering constraints to the - /// [`crate::storage_class::Output`] Storage Class memory. - OutputMemory = 0x1000, + /// Apply the memory-ordering constraints to the + /// [`crate::storage_class::Output`] Storage Class memory. + const OUTPUT_MEMORY = 0x1000; - /// Perform an availability operation on all references in the selected - /// storage classes. - MakeAvailable = 0x2000, + /// Perform an availability operation on all references in the selected + /// storage classes. + const MAKE_AVAILABLE = 0x2000; - /// Perform a visibility operation on all references in the selected - /// storage classes. - MakeVisible = 0x4000, + /// Perform a visibility operation on all references in the selected + /// storage classes. + const MAKE_VISIBLE = 0x4000; - /// This access cannot be eliminated, duplicated, or combined with - /// other accesses. - Volatile = 0x8000, + /// This access cannot be eliminated, duplicated, or combined with + /// other accesses. + const VOLATILE = 0x8000; + } } diff --git a/tests/ui/arch/control_barrier.rs b/tests/ui/arch/control_barrier.rs index c1f9d559f2..45fc687bdd 100644 --- a/tests/ui/arch/control_barrier.rs +++ b/tests/ui/arch/control_barrier.rs @@ -11,7 +11,7 @@ pub fn main() { spirv_std::arch::control_barrier::< { Scope::Subgroup }, { Scope::Subgroup }, - { Semantics::None }, + { Semantics::NONE }, >(); } } diff --git a/tests/ui/arch/memory_barrier.rs b/tests/ui/arch/memory_barrier.rs index 97810ba135..817071f06d 100644 --- a/tests/ui/arch/memory_barrier.rs +++ b/tests/ui/arch/memory_barrier.rs @@ -10,7 +10,11 @@ pub fn main() { unsafe { spirv_std::arch::memory_barrier::< { Scope::Subgroup }, - { (Semantics::AcquireRelease as u32) | (Semantics::UniformMemory as u32) }, + { + Semantics::from_bits_truncate( + Semantics::ACQUIRE_RELEASE.bits() | Semantics::UNIFORM_MEMORY.bits(), + ) + }, >(); } }