From 5294f85b4baa5fc8927798483bd3d39eee24c3b9 Mon Sep 17 00:00:00 2001 From: Zac Harrold Date: Mon, 21 Apr 2025 22:22:08 +1000 Subject: [PATCH 1/8] Switch off of `LazyLock` to a custom `RacyLock` --- naga/src/back/glsl/keywords.rs | 4 +- naga/src/back/hlsl/keywords.rs | 4 +- naga/src/back/msl/keywords.rs | 4 +- naga/src/back/pipeline_constants.rs | 4 +- naga/src/common/wgsl/to_wgsl.rs | 4 +- naga/src/keywords/wgsl.rs | 4 +- naga/src/lib.rs | 6 +-- naga/src/racy_lock.rs | 73 +++++++++++++++++++++++++++++ 8 files changed, 86 insertions(+), 17 deletions(-) create mode 100644 naga/src/racy_lock.rs diff --git a/naga/src/back/glsl/keywords.rs b/naga/src/back/glsl/keywords.rs index b8f620daed2..ed217d9e60b 100644 --- a/naga/src/back/glsl/keywords.rs +++ b/naga/src/back/glsl/keywords.rs @@ -1,4 +1,4 @@ -use std::sync::LazyLock; +use crate::racy_lock::RacyLock; use hashbrown::HashSet; @@ -499,7 +499,7 @@ pub const RESERVED_KEYWORDS: &[&str] = &[ /// significant time during [`Namer::reset`](crate::proc::Namer::reset). /// /// See for benchmarks. -pub static RESERVED_KEYWORD_SET: LazyLock> = LazyLock::new(|| { +pub static RESERVED_KEYWORD_SET: RacyLock> = RacyLock::new(|| { let mut set = HashSet::default(); set.reserve(RESERVED_KEYWORDS.len()); for &word in RESERVED_KEYWORDS { diff --git a/naga/src/back/hlsl/keywords.rs b/naga/src/back/hlsl/keywords.rs index d3b9ad8cf0d..2d0d7128228 100644 --- a/naga/src/back/hlsl/keywords.rs +++ b/naga/src/back/hlsl/keywords.rs @@ -1,4 +1,4 @@ -use std::sync::LazyLock; +use crate::racy_lock::RacyLock; use hashbrown::HashSet; @@ -924,7 +924,7 @@ pub const TYPES: &[&str] = &{ /// significant time during [`Namer::reset`](crate::proc::Namer::reset). /// /// See for benchmarks. -pub static RESERVED_SET: LazyLock> = LazyLock::new(|| { +pub static RESERVED_SET: RacyLock> = RacyLock::new(|| { let mut set = HashSet::default(); set.reserve(RESERVED.len() + TYPES.len()); for &word in RESERVED { diff --git a/naga/src/back/msl/keywords.rs b/naga/src/back/msl/keywords.rs index 4feb6004fc0..21113f5a0d1 100644 --- a/naga/src/back/msl/keywords.rs +++ b/naga/src/back/msl/keywords.rs @@ -1,4 +1,4 @@ -use std::sync::LazyLock; +use crate::racy_lock::RacyLock; use hashbrown::HashSet; @@ -360,7 +360,7 @@ pub const RESERVED: &[&str] = &[ /// significant time during [`Namer::reset`](crate::proc::Namer::reset). /// /// See for benchmarks. -pub static RESERVED_SET: LazyLock> = LazyLock::new(|| { +pub static RESERVED_SET: RacyLock> = RacyLock::new(|| { let mut set = HashSet::default(); set.reserve(RESERVED.len()); for &word in RESERVED { diff --git a/naga/src/back/pipeline_constants.rs b/naga/src/back/pipeline_constants.rs index 6c5be585204..2ab0b5ae496 100644 --- a/naga/src/back/pipeline_constants.rs +++ b/naga/src/back/pipeline_constants.rs @@ -905,7 +905,7 @@ fn map_value_to_literal(value: f64, scalar: Scalar) -> Result::trunc(value); if value < f64::from(i32::MIN) || value > f64::from(i32::MAX) { return Err(PipelineConstantError::DstRangeTooSmall); } @@ -919,7 +919,7 @@ fn map_value_to_literal(value: f64, scalar: Scalar) -> Result::trunc(value); if value < f64::from(u32::MIN) || value > f64::from(u32::MAX) { return Err(PipelineConstantError::DstRangeTooSmall); } diff --git a/naga/src/common/wgsl/to_wgsl.rs b/naga/src/common/wgsl/to_wgsl.rs index 426d15c5c41..fcc7307f498 100644 --- a/naga/src/common/wgsl/to_wgsl.rs +++ b/naga/src/common/wgsl/to_wgsl.rs @@ -14,7 +14,7 @@ use alloc::string::{String, ToString}; /// /// - If a type's WGSL form requires dynamic formatting, so that /// returning a `&'static str` isn't feasible, consider implementing -/// [`std::fmt::Display`] on some wrapper type instead. +/// [`core::fmt::Display`] on some wrapper type instead. pub trait ToWgsl: Sized { /// Return WGSL source code representation of `self`. fn to_wgsl(self) -> &'static str; @@ -32,7 +32,7 @@ pub trait ToWgsl: Sized { /// /// - If a type's WGSL form requires dynamic formatting, so that /// returning a `&'static str` isn't feasible, consider implementing -/// [`std::fmt::Display`] on some wrapper type instead. +/// [`core::fmt::Display`] on some wrapper type instead. pub trait TryToWgsl: Sized { /// Return the WGSL form of `self` as a `'static` string. /// diff --git a/naga/src/keywords/wgsl.rs b/naga/src/keywords/wgsl.rs index 82be25ed4df..df464a10ff4 100644 --- a/naga/src/keywords/wgsl.rs +++ b/naga/src/keywords/wgsl.rs @@ -4,7 +4,7 @@ Keywords for [WGSL][wgsl] (WebGPU Shading Language). [wgsl]: https://gpuweb.github.io/gpuweb/wgsl.html */ -use std::sync::LazyLock; +use crate::racy_lock::RacyLock; use hashbrown::HashSet; @@ -238,7 +238,7 @@ pub const RESERVED: &[&str] = &[ /// significant time during [`Namer::reset`](crate::proc::Namer::reset). /// /// See for benchmarks. -pub static RESERVED_SET: LazyLock> = LazyLock::new(|| { +pub static RESERVED_SET: RacyLock> = RacyLock::new(|| { let mut set = HashSet::default(); set.reserve(RESERVED.len()); for &word in RESERVED { diff --git a/naga/src/lib.rs b/naga/src/lib.rs index d440a61af79..ef9cfd90799 100644 --- a/naga/src/lib.rs +++ b/naga/src/lib.rs @@ -104,11 +104,6 @@ void main() { test, spv_out, - // Need OnceLock - hlsl_out, - msl_out, - wgsl_out, - feature = "spv-in", feature = "wgsl-in", @@ -130,6 +125,7 @@ pub mod ir; pub mod keywords; mod non_max_u32; pub mod proc; +mod racy_lock; mod span; pub mod valid; diff --git a/naga/src/racy_lock.rs b/naga/src/racy_lock.rs new file mode 100644 index 00000000000..59973c4abbf --- /dev/null +++ b/naga/src/racy_lock.rs @@ -0,0 +1,73 @@ +#![cfg_attr( + not(any(hlsl_out, msl_out, wgsl_out, glsl_out)), + allow(dead_code, reason = "RacyLock is only required for the above configurations") +)] + +use alloc::boxed::Box; +use core::sync::atomic::{AtomicPtr, Ordering}; + +/// An alternative to [`LazyLock`] which will race to initialize rather than blocking. +/// This makes it suitable for `no_std` environments, at the expense of possibly leaking +/// memory during initialization. +/// +/// [`LazyLock`]: https://doc.rust-lang.org/stable/std/sync/struct.LazyLock.html +pub struct RacyLock { + inner: AtomicPtr, + init: fn() -> T, +} + +impl RacyLock { + /// Creates a new [`RacyLock`], which will initialize using the provided `init` function. + pub const fn new(init: fn() -> T) -> Self { + Self { + inner: AtomicPtr::new(core::ptr::null_mut()), + init, + } + } + + /// Attempts to load the internal value, returning [`None`] if it is not yet initialized. + pub fn try_get(&self) -> Option<&T> { + let ptr = self.inner.load(Ordering::Acquire); + + if ptr.is_null() { + None + } else { + // SAFETY: ptr can only ever be null, or a static-valid value from Box::leak, + // as it is private. + // The above check ensures ptr is not null, so it must be a valid pointer. + unsafe { Some(&*ptr) } + } + } + + /// Loads the internal value, initializing it if required. + pub fn get(&self) -> &T { + self.try_get().unwrap_or_else(|| { + let value = (self.init)(); + + // Refresh the static value just before leaking to minimize leaked memory. + let ptr = self.inner.load(Ordering::Acquire); + + if ptr.is_null() { + // Explicit type used to assert the returned reference is 'static. + let ptr: &'static mut T = Box::leak(Box::new(value)); + + self.inner.store(ptr, Ordering::Release); + + ptr + } else { + // SAFETY: ptr can only ever be null, or a static-valid value from Box::leak, + // as it is private. + // The above check ensures ptr is not null, so it must be a valid pointer. + unsafe { &*ptr } + } + }) + } +} + +impl core::ops::Deref for RacyLock { + type Target = T; + + fn deref(&self) -> &Self::Target { + self.get() + } +} From 263ea2037db771fbb529ff9c31fa775e8915432e Mon Sep 17 00:00:00 2001 From: Zac Harrold Date: Mon, 21 Apr 2025 22:42:32 +1000 Subject: [PATCH 2/8] Formatting --- naga/src/lib.rs | 2 -- naga/src/racy_lock.rs | 5 ++++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/naga/src/lib.rs b/naga/src/lib.rs index ef9cfd90799..145f220334c 100644 --- a/naga/src/lib.rs +++ b/naga/src/lib.rs @@ -103,10 +103,8 @@ void main() { #[cfg(any( test, spv_out, - feature = "spv-in", feature = "wgsl-in", - feature = "stderr", ))] extern crate std; diff --git a/naga/src/racy_lock.rs b/naga/src/racy_lock.rs index 59973c4abbf..082fbc416d6 100644 --- a/naga/src/racy_lock.rs +++ b/naga/src/racy_lock.rs @@ -1,6 +1,9 @@ #![cfg_attr( not(any(hlsl_out, msl_out, wgsl_out, glsl_out)), - allow(dead_code, reason = "RacyLock is only required for the above configurations") + allow( + dead_code, + reason = "RacyLock is only required for the above configurations" + ) )] use alloc::boxed::Box; From a19fd0aa4a71027d2ba2aa1c096316d824fdd9ca Mon Sep 17 00:00:00 2001 From: Zac Harrold Date: Tue, 22 Apr 2025 19:01:53 +1000 Subject: [PATCH 3/8] Switch to `OnceBox` internally --- naga/src/racy_lock.rs | 38 +++++--------------------------------- 1 file changed, 5 insertions(+), 33 deletions(-) diff --git a/naga/src/racy_lock.rs b/naga/src/racy_lock.rs index 082fbc416d6..cd725205d44 100644 --- a/naga/src/racy_lock.rs +++ b/naga/src/racy_lock.rs @@ -7,7 +7,7 @@ )] use alloc::boxed::Box; -use core::sync::atomic::{AtomicPtr, Ordering}; +use once_cell::race::OnceBox; /// An alternative to [`LazyLock`] which will race to initialize rather than blocking. /// This makes it suitable for `no_std` environments, at the expense of possibly leaking @@ -15,7 +15,7 @@ use core::sync::atomic::{AtomicPtr, Ordering}; /// /// [`LazyLock`]: https://doc.rust-lang.org/stable/std/sync/struct.LazyLock.html pub struct RacyLock { - inner: AtomicPtr, + inner: OnceBox, init: fn() -> T, } @@ -23,47 +23,19 @@ impl RacyLock { /// Creates a new [`RacyLock`], which will initialize using the provided `init` function. pub const fn new(init: fn() -> T) -> Self { Self { - inner: AtomicPtr::new(core::ptr::null_mut()), + inner: OnceBox::new(), init, } } /// Attempts to load the internal value, returning [`None`] if it is not yet initialized. pub fn try_get(&self) -> Option<&T> { - let ptr = self.inner.load(Ordering::Acquire); - - if ptr.is_null() { - None - } else { - // SAFETY: ptr can only ever be null, or a static-valid value from Box::leak, - // as it is private. - // The above check ensures ptr is not null, so it must be a valid pointer. - unsafe { Some(&*ptr) } - } + self.inner.get() } /// Loads the internal value, initializing it if required. pub fn get(&self) -> &T { - self.try_get().unwrap_or_else(|| { - let value = (self.init)(); - - // Refresh the static value just before leaking to minimize leaked memory. - let ptr = self.inner.load(Ordering::Acquire); - - if ptr.is_null() { - // Explicit type used to assert the returned reference is 'static. - let ptr: &'static mut T = Box::leak(Box::new(value)); - - self.inner.store(ptr, Ordering::Release); - - ptr - } else { - // SAFETY: ptr can only ever be null, or a static-valid value from Box::leak, - // as it is private. - // The above check ensures ptr is not null, so it must be a valid pointer. - unsafe { &*ptr } - } - }) + self.inner.get_or_init(|| Box::new((self.init)())) } } From 7c70c076a16e7fbc925bbc54298913073b8b7d7e Mon Sep 17 00:00:00 2001 From: Zac Harrold Date: Tue, 22 Apr 2025 19:03:18 +1000 Subject: [PATCH 4/8] Clippy --- naga/src/racy_lock.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/naga/src/racy_lock.rs b/naga/src/racy_lock.rs index cd725205d44..9cd08397302 100644 --- a/naga/src/racy_lock.rs +++ b/naga/src/racy_lock.rs @@ -28,11 +28,6 @@ impl RacyLock { } } - /// Attempts to load the internal value, returning [`None`] if it is not yet initialized. - pub fn try_get(&self) -> Option<&T> { - self.inner.get() - } - /// Loads the internal value, initializing it if required. pub fn get(&self) -> &T { self.inner.get_or_init(|| Box::new((self.init)())) From 92a1294c1f5de8857cc14aaa8f54e7b3bb6133c6 Mon Sep 17 00:00:00 2001 From: Zac Harrold Date: Wed, 23 Apr 2025 09:48:51 +1000 Subject: [PATCH 5/8] Simplify `trunc` usage Co-Authored-By: Connor Fitzgerald --- naga/build.rs | 2 ++ naga/src/back/pipeline_constants.rs | 7 +++++-- naga/src/lib.rs | 8 +------- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/naga/build.rs b/naga/build.rs index e647f1fdd0a..38b1a28a869 100644 --- a/naga/build.rs +++ b/naga/build.rs @@ -6,5 +6,7 @@ fn main() { msl_out: { any(feature = "msl-out", all(target_vendor = "apple", feature = "msl-out-if-target-apple")) }, spv_out: { feature = "spv-out" }, wgsl_out: { feature = "wgsl-out" }, + std: { any(test, spv_out, feature = "spv-in", feature = "wgsl-in", feature = "stderr") }, + no_std: { not(std) }, } } diff --git a/naga/src/back/pipeline_constants.rs b/naga/src/back/pipeline_constants.rs index 2ab0b5ae496..1cf1c805249 100644 --- a/naga/src/back/pipeline_constants.rs +++ b/naga/src/back/pipeline_constants.rs @@ -16,6 +16,9 @@ use crate::{ Span, Statement, TypeInner, WithSpan, }; +#[cfg(no_std)] +use num_traits::float::FloatCore as _; + #[derive(Error, Debug, Clone)] #[cfg_attr(test, derive(PartialEq))] pub enum PipelineConstantError { @@ -905,7 +908,7 @@ fn map_value_to_literal(value: f64, scalar: Scalar) -> Result::trunc(value); + let value = value.trunc(); if value < f64::from(i32::MIN) || value > f64::from(i32::MAX) { return Err(PipelineConstantError::DstRangeTooSmall); } @@ -919,7 +922,7 @@ fn map_value_to_literal(value: f64, scalar: Scalar) -> Result::trunc(value); + let value = value.trunc(); if value < f64::from(u32::MIN) || value > f64::from(u32::MAX) { return Err(PipelineConstantError::DstRangeTooSmall); } diff --git a/naga/src/lib.rs b/naga/src/lib.rs index 145f220334c..d6229742b20 100644 --- a/naga/src/lib.rs +++ b/naga/src/lib.rs @@ -100,13 +100,7 @@ void main() { )] #![no_std] -#[cfg(any( - test, - spv_out, - feature = "spv-in", - feature = "wgsl-in", - feature = "stderr", -))] +#[cfg(std)] extern crate std; extern crate alloc; From 3c5178c39a0644b99c9fba1a2025121d5751a9ba Mon Sep 17 00:00:00 2001 From: Zac Harrold Date: Wed, 23 Apr 2025 09:53:04 +1000 Subject: [PATCH 6/8] Switch to `expect` Co-Authored-By: Connor Fitzgerald --- naga/src/racy_lock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/naga/src/racy_lock.rs b/naga/src/racy_lock.rs index 9cd08397302..fdcd849116b 100644 --- a/naga/src/racy_lock.rs +++ b/naga/src/racy_lock.rs @@ -1,6 +1,6 @@ #![cfg_attr( not(any(hlsl_out, msl_out, wgsl_out, glsl_out)), - allow( + expect( dead_code, reason = "RacyLock is only required for the above configurations" ) From 8b5d0ca5e2a691c9b9c2e564403521ff52322c31 Mon Sep 17 00:00:00 2001 From: Zac Harrold Date: Wed, 23 Apr 2025 10:05:55 +1000 Subject: [PATCH 7/8] Fix expectation conditions Co-Authored-By: Connor Fitzgerald --- naga/src/racy_lock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/naga/src/racy_lock.rs b/naga/src/racy_lock.rs index fdcd849116b..72b7b478d8b 100644 --- a/naga/src/racy_lock.rs +++ b/naga/src/racy_lock.rs @@ -1,5 +1,5 @@ #![cfg_attr( - not(any(hlsl_out, msl_out, wgsl_out, glsl_out)), + not(any(glsl_out, hlsl_out, msl_out, feature = "wgsl-in", wgsl_out)), expect( dead_code, reason = "RacyLock is only required for the above configurations" From 2e8e40430a1bf108ea51fe375533b3036af67276 Mon Sep 17 00:00:00 2001 From: Zac Harrold Date: Wed, 30 Apr 2025 19:10:20 +1000 Subject: [PATCH 8/8] Update documentation --- naga/src/racy_lock.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/naga/src/racy_lock.rs b/naga/src/racy_lock.rs index 72b7b478d8b..116808055a6 100644 --- a/naga/src/racy_lock.rs +++ b/naga/src/racy_lock.rs @@ -9,9 +9,7 @@ use alloc::boxed::Box; use once_cell::race::OnceBox; -/// An alternative to [`LazyLock`] which will race to initialize rather than blocking. -/// This makes it suitable for `no_std` environments, at the expense of possibly leaking -/// memory during initialization. +/// An alternative to [`LazyLock`] based on [`OnceBox`]. /// /// [`LazyLock`]: https://doc.rust-lang.org/stable/std/sync/struct.LazyLock.html pub struct RacyLock {