From eae40383d08d2d63af54d0f6990f471c74a96ac3 Mon Sep 17 00:00:00 2001
From: Dzmitry Malyshau <kvark@fastmail.com>
Date: Thu, 11 Feb 2021 21:17:36 -0500
Subject: [PATCH] Add uniform control flow analysis to the validator

---
 src/proc/analyzer.rs  | 27 +++++++++++++++++----------
 src/proc/validator.rs | 26 ++++++++++++++++----------
 tests/parse.rs        |  2 +-
 3 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/src/proc/analyzer.rs b/src/proc/analyzer.rs
index 13d9d36918..6ff4c70fcc 100644
--- a/src/proc/analyzer.rs
+++ b/src/proc/analyzer.rs
@@ -79,19 +79,26 @@ impl FunctionInfo {
             E::FunctionArgument(_) => ControlFlags::NON_UNIFORM_RESULT, //TODO?
             E::GlobalVariable(handle) => {
                 let var = &global_var_arena[handle];
-                match var.binding {
-                    Some(crate::Binding::BuiltIn(built_in)) => match built_in {
+                let uniform = if let Some(crate::Binding::BuiltIn(built_in)) = var.binding {
+                    match built_in {
                         crate::BuiltIn::FrontFacing
                         | crate::BuiltIn::WorkGroupId
-                        | crate::BuiltIn::WorkGroupSize => ControlFlags::empty(),
-                        _ => ControlFlags::NON_UNIFORM_RESULT,
-                    },
-                    _ if var.class == crate::StorageClass::Input
-                        && var.interpolation == Some(crate::Interpolation::Flat) =>
-                    {
-                        ControlFlags::empty()
+                        | crate::BuiltIn::WorkGroupSize => true,
+                        _ => false,
                     }
-                    _ => ControlFlags::NON_UNIFORM_RESULT,
+                } else {
+                    use crate::StorageClass as Sc;
+                    match var.class {
+                        Sc::Input => var.interpolation == Some(crate::Interpolation::Flat),
+                        Sc::Output | Sc::Function | Sc::Private | Sc::WorkGroup => false,
+                        Sc::Uniform | Sc::Handle | Sc::PushConstant => true,
+                        Sc::Storage => !var.storage_access.contains(crate::StorageAccess::STORE),
+                    }
+                };
+                if uniform {
+                    ControlFlags::empty()
+                } else {
+                    ControlFlags::NON_UNIFORM_RESULT
                 }
             }
             E::LocalVariable(_) => {
diff --git a/src/proc/validator.rs b/src/proc/validator.rs
index 628a5c48cf..a3e7fd15e7 100644
--- a/src/proc/validator.rs
+++ b/src/proc/validator.rs
@@ -1,4 +1,7 @@
-use super::typifier::{ResolveContext, ResolveError, Typifier};
+use super::{
+    analyzer::{Analysis, AnalysisError},
+    typifier::{ResolveContext, ResolveError, Typifier},
+};
 use crate::arena::{Arena, Handle};
 use bit_set::BitSet;
 
@@ -23,7 +26,7 @@ pub struct Validator {
     bind_group_masks: Vec<BitSet>,
 }
 
-#[derive(Clone, Debug, PartialEq, thiserror::Error)]
+#[derive(Clone, Debug, thiserror::Error)]
 pub enum TypeError {
     #[error("The {0:?} scalar width {1} is not supported")]
     InvalidWidth(crate::ScalarKind, crate::Bytes),
@@ -37,7 +40,7 @@ pub enum TypeError {
     MissingBlockDecoration,
 }
 
-#[derive(Clone, Debug, PartialEq, thiserror::Error)]
+#[derive(Clone, Debug, thiserror::Error)]
 pub enum ConstantError {
     #[error("The type doesn't match the constant")]
     InvalidType,
@@ -47,7 +50,7 @@ pub enum ConstantError {
     UnresolvedSize(Handle<crate::Constant>),
 }
 
-#[derive(Clone, Debug, PartialEq, thiserror::Error)]
+#[derive(Clone, Debug, thiserror::Error)]
 pub enum GlobalVariableError {
     #[error("Usage isn't compatible with the storage class")]
     InvalidUsage,
@@ -66,13 +69,13 @@ pub enum GlobalVariableError {
     InvalidBuiltInType(crate::BuiltIn),
 }
 
-#[derive(Clone, Debug, PartialEq, thiserror::Error)]
+#[derive(Clone, Debug, thiserror::Error)]
 pub enum LocalVariableError {
     #[error("Initializer doesn't match the variable type")]
     InitializerType,
 }
 
-#[derive(Clone, Debug, PartialEq, thiserror::Error)]
+#[derive(Clone, Debug, thiserror::Error)]
 pub enum FunctionError {
     #[error(transparent)]
     Resolve(#[from] ResolveError),
@@ -90,7 +93,7 @@ pub enum FunctionError {
     InvalidArgumentType { index: usize, name: String },
 }
 
-#[derive(Clone, Debug, PartialEq, thiserror::Error)]
+#[derive(Clone, Debug, thiserror::Error)]
 pub enum EntryPointError {
     #[error("Early depth test is not applicable")]
     UnexpectedEarlyDepthTest,
@@ -114,7 +117,7 @@ pub enum EntryPointError {
     Function(#[from] FunctionError),
 }
 
-#[derive(Clone, Debug, PartialEq, thiserror::Error)]
+#[derive(Clone, Debug, thiserror::Error)]
 pub enum ValidationError {
     #[error("Type {handle:?} '{name}' is invalid: {error:?}")]
     Type {
@@ -142,6 +145,8 @@ pub enum ValidationError {
         name: String,
         error: EntryPointError,
     },
+    #[error(transparent)]
+    Analysis(#[from] AnalysisError),
     #[error("Module is corrupted")]
     Corrupted,
 }
@@ -690,7 +695,7 @@ impl Validator {
     }
 
     /// Check the given module to be valid.
-    pub fn validate(&mut self, module: &crate::Module) -> Result<(), ValidationError> {
+    pub fn validate(&mut self, module: &crate::Module) -> Result<Analysis, ValidationError> {
         self.typifier.clear();
         self.type_flags.clear();
         self.type_flags
@@ -742,6 +747,7 @@ impl Validator {
                 })?;
         }
 
-        Ok(())
+        let analysis = Analysis::new(module)?;
+        Ok(analysis)
     }
 }
diff --git a/tests/parse.rs b/tests/parse.rs
index 9e736d7ec5..e9d0cdf9bc 100644
--- a/tests/parse.rs
+++ b/tests/parse.rs
@@ -16,7 +16,7 @@ fn check_glsl(name: &str) {
 
     match naga::front::glsl::parse_str(&input, "main", stage, Default::default()) {
         Ok(m) => match naga::proc::Validator::new().validate(&m) {
-            Ok(()) => (),
+            Ok(_analysis) => (),
             Err(e) => panic!("Unable to validate {}: {:?}", name, e),
         },
         Err(e) => panic!("Unable to parse {}: {:?}", name, e),