From dcca4e33a196413bf0c921869491aa3d48cbc0a7 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Fri, 18 Oct 2024 17:19:19 -0400 Subject: [PATCH 1/2] feat: implement `requires` directives --- CHANGELOG.md | 2 +- naga/src/front/wgsl/error.rs | 32 +++++++ naga/src/front/wgsl/parse/directive.rs | 24 ++---- .../parse/directive/language_extension.rs | 85 +++++++++++++++++++ naga/src/front/wgsl/parse/mod.rs | 18 ++++ 5 files changed, 142 insertions(+), 19 deletions(-) create mode 100644 naga/src/front/wgsl/parse/directive/language_extension.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index ef115f7cc4..aa327cac88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,7 +85,7 @@ By @bradwerth [#6216](https://github.com/gfx-rs/wgpu/pull/6216). - Support local `const` declarations in WGSL. By @sagudev in [#6156](https://github.com/gfx-rs/wgpu/pull/6156). - Implemented `const_assert` in WGSL. By @sagudev in [#6198](https://github.com/gfx-rs/wgpu/pull/6198). - Support polyfilling `inverse` in WGSL. By @chyyran in [#6385](https://github.com/gfx-rs/wgpu/pull/6385). -- Add an internal skeleton for parsing `requires`, `enable`, and `diagnostic` directives that don't yet do anything besides emit nicer errors. By @ErichDonGubler in [#6352](https://github.com/gfx-rs/wgpu/pull/6352), [#6424](https://github.com/gfx-rs/wgpu/pull/6424). +- Add an internal skeleton for parsing `requires`, `enable`, and `diagnostic` directives that don't yet do anything besides emit nicer errors. By @ErichDonGubler in [#6352](https://github.com/gfx-rs/wgpu/pull/6352), [#6424](https://github.com/gfx-rs/wgpu/pull/6424), [#6437](https://github.com/gfx-rs/wgpu/pull/6437). - Include error chain information as a message and notes in shader compilation messages. By @ErichDonGubler in [#6436](https://github.com/gfx-rs/wgpu/pull/6436). - Unify Naga CLI error output with the format of shader compilation messages. By @ErichDonGubler in [#6436](https://github.com/gfx-rs/wgpu/pull/6436). diff --git a/naga/src/front/wgsl/error.rs b/naga/src/front/wgsl/error.rs index eeab34d47c..b19aad506d 100644 --- a/naga/src/front/wgsl/error.rs +++ b/naga/src/front/wgsl/error.rs @@ -1,6 +1,9 @@ use crate::front::wgsl::parse::directive::enable_extension::{ EnableExtension, UnimplementedEnableExtension, }; +use crate::front::wgsl::parse::directive::language_extension::{ + LanguageExtension, UnimplementedLanguageExtension, +}; use crate::front::wgsl::parse::directive::{DirectiveKind, UnimplementedDirectiveKind}; use crate::front::wgsl::parse::lexer::Token; use crate::front::wgsl::Scalar; @@ -190,6 +193,7 @@ pub(crate) enum Error<'a> { UnknownStorageFormat(Span), UnknownConservativeDepth(Span), UnknownEnableExtension(Span, &'a str), + UnknownLanguageExtension(Span, &'a str), SizeAttributeTooLow(Span, u32), AlignAttributeTooLow(Span, Alignment), NonPowerOfTwoAlignAttribute(Span), @@ -287,6 +291,10 @@ pub(crate) enum Error<'a> { kind: EnableExtension, span: Span, }, + LanguageExtensionNotYetImplemented { + kind: UnimplementedLanguageExtension, + span: Span, + }, } #[derive(Clone, Debug)] @@ -545,6 +553,15 @@ impl<'a> Error<'a> { .into(), ], }, + Error::UnknownLanguageExtension(span, name) => ParseError { + message: format!("unknown language extension `{name}`"), + labels: vec![(span, "".into())], + notes: vec![concat!( + "See available extensions at ", + "." + ) + .into()], + }, Error::SizeAttributeTooLow(bad_span, min_size) => ParseError { message: format!("struct member size must be at least {min_size}"), labels: vec![(bad_span, format!("must be at least {min_size}").into())], @@ -976,6 +993,21 @@ impl<'a> Error<'a> { vec![] }, }, + Error::LanguageExtensionNotYetImplemented { kind, span } => ParseError { + message: format!( + "the `{}` language extension is not yet supported", + LanguageExtension::Unimplemented(kind).to_ident() + ), + labels: vec![(span, "".into())], + notes: vec![format!( + concat!( + "Let Naga maintainers know that you ran into this at ", + ", ", + "so they can prioritize it!" + ), + kind.tracking_issue_num() + )], + }, } } } diff --git a/naga/src/front/wgsl/parse/directive.rs b/naga/src/front/wgsl/parse/directive.rs index 1627f1288d..6486c8bb2d 100644 --- a/naga/src/front/wgsl/parse/directive.rs +++ b/naga/src/front/wgsl/parse/directive.rs @@ -3,12 +3,15 @@ //! See also . pub mod enable_extension; +pub(crate) mod language_extension; /// A parsed sentinel word indicating the type of directive to be parsed next. #[derive(Clone, Copy, Debug, Hash, Eq, PartialEq)] pub enum DirectiveKind { /// An [`enable_extension`]. Enable, + /// A language extension. + Requires, Unimplemented(UnimplementedDirectiveKind), } @@ -22,7 +25,7 @@ impl DirectiveKind { Some(match s { Self::DIAGNOSTIC => Self::Unimplemented(UnimplementedDirectiveKind::Diagnostic), Self::ENABLE => Self::Enable, - Self::REQUIRES => Self::Unimplemented(UnimplementedDirectiveKind::Requires), + Self::REQUIRES => Self::Requires, _ => return None, }) } @@ -31,9 +34,9 @@ impl DirectiveKind { pub const fn to_ident(self) -> &'static str { match self { Self::Enable => Self::ENABLE, + Self::Requires => Self::REQUIRES, Self::Unimplemented(kind) => match kind { UnimplementedDirectiveKind::Diagnostic => Self::DIAGNOSTIC, - UnimplementedDirectiveKind::Requires => Self::REQUIRES, }, } } @@ -51,14 +54,12 @@ impl DirectiveKind { #[cfg_attr(test, derive(strum::EnumIter))] pub enum UnimplementedDirectiveKind { Diagnostic, - Requires, } impl UnimplementedDirectiveKind { pub const fn tracking_issue_num(self) -> u16 { match self { Self::Diagnostic => 5320, - Self::Requires => 6350, } } } @@ -88,19 +89,6 @@ error: `diagnostic` is not yet implemented │ = note: Let Naga maintainers know that you ran into this at , so they can prioritize it! -"; - } - UnimplementedDirectiveKind::Requires => { - shader = "requires readonly_and_readwrite_storage_textures"; - expected_msg = "\ -error: `requires` is not yet implemented - ┌─ wgsl:1:1 - │ -1 │ requires readonly_and_readwrite_storage_textures - │ ^^^^^^^^ this global directive is standard, but not yet implemented - │ - = note: Let Naga maintainers know that you ran into this at , so they can prioritize it! - "; } }; @@ -141,7 +129,7 @@ error: expected global declaration, but found a global directive "; } - DirectiveKind::Unimplemented(UnimplementedDirectiveKind::Requires) => { + DirectiveKind::Requires => { directive = "requires readonly_and_readwrite_storage_textures"; expected_msg = "\ error: expected global declaration, but found a global directive diff --git a/naga/src/front/wgsl/parse/directive/language_extension.rs b/naga/src/front/wgsl/parse/directive/language_extension.rs new file mode 100644 index 0000000000..a80e7aae98 --- /dev/null +++ b/naga/src/front/wgsl/parse/directive/language_extension.rs @@ -0,0 +1,85 @@ +//! `requires …;` extensions in WGSL. +//! +//! The focal point of this module is the [`LanguageExtension`] API. + +/// A language extension not guaranteed to be present in all environments. +/// +/// WGSL spec.: +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub(crate) enum LanguageExtension { + #[allow(unused)] + Implemented(ImplementedLanguageExtension), + Unimplemented(UnimplementedLanguageExtension), +} + +impl LanguageExtension { + const READONLY_AND_READWRITE_STORAGE_TEXTURES: &'static str = + "readonly_and_readwrite_storage_textures"; + const PACKED4X8_INTEGER_DOT_PRODUCT: &'static str = "packed_4x8_integer_dot_product"; + const UNRESTRICTED_POINTER_PARAMETERS: &'static str = "unrestricted_pointer_parameters"; + const POINTER_COMPOSITE_ACCESS: &'static str = "pointer_composite_access"; + + /// Convert from a sentinel word in WGSL into its associated [`LanguageExtension`], if possible. + pub fn from_ident(s: &str) -> Option { + Some(match s { + Self::READONLY_AND_READWRITE_STORAGE_TEXTURES => Self::Unimplemented( + UnimplementedLanguageExtension::ReadOnlyAndReadWriteStorageTextures, + ), + Self::PACKED4X8_INTEGER_DOT_PRODUCT => { + Self::Unimplemented(UnimplementedLanguageExtension::Packed4x8IntegerDotProduct) + } + Self::UNRESTRICTED_POINTER_PARAMETERS => { + Self::Unimplemented(UnimplementedLanguageExtension::UnrestrictedPointerParameters) + } + Self::POINTER_COMPOSITE_ACCESS => { + Self::Unimplemented(UnimplementedLanguageExtension::PointerCompositeAccess) + } + _ => return None, + }) + } + + /// Maps this [`LanguageExtension`] into the sentinel word associated with it in WGSL. + pub const fn to_ident(self) -> &'static str { + match self { + Self::Implemented(kind) => match kind {}, + Self::Unimplemented(kind) => match kind { + UnimplementedLanguageExtension::ReadOnlyAndReadWriteStorageTextures => { + Self::READONLY_AND_READWRITE_STORAGE_TEXTURES + } + UnimplementedLanguageExtension::Packed4x8IntegerDotProduct => { + Self::PACKED4X8_INTEGER_DOT_PRODUCT + } + UnimplementedLanguageExtension::UnrestrictedPointerParameters => { + Self::UNRESTRICTED_POINTER_PARAMETERS + } + UnimplementedLanguageExtension::PointerCompositeAccess => { + Self::POINTER_COMPOSITE_ACCESS + } + }, + } + } +} + +/// A variant of [`LanguageExtension::Implemented`]. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub(crate) enum ImplementedLanguageExtension {} + +/// A variant of [`LanguageExtension::Unimplemented`]. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub(crate) enum UnimplementedLanguageExtension { + ReadOnlyAndReadWriteStorageTextures, + Packed4x8IntegerDotProduct, + UnrestrictedPointerParameters, + PointerCompositeAccess, +} + +impl UnimplementedLanguageExtension { + pub(crate) const fn tracking_issue_num(self) -> u16 { + match self { + Self::ReadOnlyAndReadWriteStorageTextures => 6204, + Self::Packed4x8IntegerDotProduct => 6445, + Self::UnrestrictedPointerParameters => 5158, + Self::PointerCompositeAccess => 6192, + } + } +} diff --git a/naga/src/front/wgsl/parse/mod.rs b/naga/src/front/wgsl/parse/mod.rs index ca35015943..fcfcc37750 100644 --- a/naga/src/front/wgsl/parse/mod.rs +++ b/naga/src/front/wgsl/parse/mod.rs @@ -2,6 +2,7 @@ use crate::front::wgsl::error::{Error, ExpectedToken}; use crate::front::wgsl::parse::directive::enable_extension::{ EnableExtension, EnableExtensions, UnimplementedEnableExtension, }; +use crate::front::wgsl::parse::directive::language_extension::LanguageExtension; use crate::front::wgsl::parse::directive::DirectiveKind; use crate::front::wgsl::parse::lexer::{Lexer, Token}; use crate::front::wgsl::parse::number::Number; @@ -2544,6 +2545,23 @@ impl Parser { Ok(()) })?; } + DirectiveKind::Requires => { + self.directive_ident_list(&mut lexer, |ident, span| { + match LanguageExtension::from_ident(ident) { + Some(LanguageExtension::Implemented(_kind)) => { + // NOTE: No further validation is needed for an extension, so + // just throw parsed information away. If we ever want to apply + // what we've parsed to diagnostics, maybe we'll want to refer + // to enabled extensions later? + Ok(()) + } + Some(LanguageExtension::Unimplemented(kind)) => { + Err(Error::LanguageExtensionNotYetImplemented { kind, span }) + } + None => Err(Error::UnknownLanguageExtension(span, ident)), + } + })?; + } DirectiveKind::Unimplemented(kind) => { return Err(Error::DirectiveNotYetImplemented { kind, span }) } From c1fe4cacf53db830bbd859f662581936db0fba8d Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Tue, 22 Oct 2024 18:16:56 -0400 Subject: [PATCH 2/2] docs(CHANGELOG): re-word directive parsing item a bit --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa327cac88..148564d6a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,7 +85,7 @@ By @bradwerth [#6216](https://github.com/gfx-rs/wgpu/pull/6216). - Support local `const` declarations in WGSL. By @sagudev in [#6156](https://github.com/gfx-rs/wgpu/pull/6156). - Implemented `const_assert` in WGSL. By @sagudev in [#6198](https://github.com/gfx-rs/wgpu/pull/6198). - Support polyfilling `inverse` in WGSL. By @chyyran in [#6385](https://github.com/gfx-rs/wgpu/pull/6385). -- Add an internal skeleton for parsing `requires`, `enable`, and `diagnostic` directives that don't yet do anything besides emit nicer errors. By @ErichDonGubler in [#6352](https://github.com/gfx-rs/wgpu/pull/6352), [#6424](https://github.com/gfx-rs/wgpu/pull/6424), [#6437](https://github.com/gfx-rs/wgpu/pull/6437). +- Add base support for parsing `requires`, `enable`, and `diagnostic` directives. No extensions or diagnostic filters are yet supported, but diagnostics have improved dramatically. By @ErichDonGubler in [#6352](https://github.com/gfx-rs/wgpu/pull/6352), [#6424](https://github.com/gfx-rs/wgpu/pull/6424), [#6437](https://github.com/gfx-rs/wgpu/pull/6437). - Include error chain information as a message and notes in shader compilation messages. By @ErichDonGubler in [#6436](https://github.com/gfx-rs/wgpu/pull/6436). - Unify Naga CLI error output with the format of shader compilation messages. By @ErichDonGubler in [#6436](https://github.com/gfx-rs/wgpu/pull/6436).