Merge two implementations of ValidateExecutionScope (#2131)

Android.mk

@@ -68,6 +68,7 @@ SPVTOOLS_SRC_FILES := \
 		source/val/validate_logicals.cpp \
 		source/val/validate_non_uniform.cpp \
 		source/val/validate_primitives.cpp \
+		source/val/validate_scopes.cpp \
 		source/val/validate_type.cpp
 
 SPVTOOLS_OPT_SRC_FILES := \

BUILD.gn

@@ -424,6 +424,7 @@ static_library("spvtools_val") {
     "source/val/validate_mode_setting.cpp",
     "source/val/validate_non_uniform.cpp",
     "source/val/validate_primitives.cpp",
+    "source/val/validate_scopes.cpp",
     "source/val/validate_type.cpp",
     "source/val/validation_state.cpp",
   ]

source/CMakeLists.txt

@@ -319,6 +319,7 @@ set(SPIRV_SOURCES
   ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_mode_setting.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_non_uniform.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_primitives.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_scopes.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_type.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/val/decoration.h
   ${CMAKE_CURRENT_SOURCE_DIR}/val/basic_block.cpp

source/val/validate_barriers.cpp

@@ -24,65 +24,13 @@
 #include "source/spirv_target_env.h"
 #include "source/util/bitutils.h"
 #include "source/val/instruction.h"
+#include "source/val/validate_scopes.h"
 #include "source/val/validation_state.h"
 
 namespace spvtools {
 namespace val {
 namespace {
 
-// Validates Execution Scope operand.
-spv_result_t ValidateExecutionScope(ValidationState_t& _,
-                                    const Instruction* inst, uint32_t id) {
-  const SpvOp opcode = inst->opcode();
-  bool is_int32 = false, is_const_int32 = false;
-  uint32_t value = 0;
-  std::tie(is_int32, is_const_int32, value) = _.EvalInt32IfConst(id);
-
-  if (!is_int32) {
-    return _.diag(SPV_ERROR_INVALID_DATA, inst)
-           << spvOpcodeString(opcode)
-           << ": expected Execution Scope to be a 32-bit int";
-  }
-
-  if (!is_const_int32) {
-    return SPV_SUCCESS;
-  }
-
-  if (spvIsVulkanEnv(_.context()->target_env)) {
-    if (value != SpvScopeWorkgroup && value != SpvScopeSubgroup) {
-      return _.diag(SPV_ERROR_INVALID_DATA, inst)
-             << spvOpcodeString(opcode)
-             << ": in Vulkan environment Execution Scope is limited to "
-                "Workgroup and Subgroup";
-    }
-
-    if (_.context()->target_env != SPV_ENV_VULKAN_1_0 &&
-        value != SpvScopeSubgroup) {
-      _.function(inst->function()->id())
-          ->RegisterExecutionModelLimitation([](SpvExecutionModel model,
-                                                std::string* message) {
-            if (model == SpvExecutionModelFragment ||
-                model == SpvExecutionModelVertex ||
-                model == SpvExecutionModelGeometry ||
-                model == SpvExecutionModelTessellationEvaluation) {
-              if (message) {
-                *message =
-                    "in Vulkan evironment, OpControlBarrier execution scope "
-                    "must be Subgroup for Fragment, Vertex, Geometry and "
-                    "TessellationEvaluation execution models";
-              }
-              return false;
-            }
-            return true;
-          });
-    }
-  }
-
-  // TODO(atgoo@github.com) Add checks for OpenCL and OpenGL environments.
-
-  return SPV_SUCCESS;
-}
-
 // Validates Memory Scope operand.
 spv_result_t ValidateMemoryScope(ValidationState_t& _, const Instruction* inst,
                                  uint32_t id) {

source/val/validate_non_uniform.cpp

@@ -22,47 +22,13 @@
 #include "source/spirv_target_env.h"
 #include "source/util/bitutils.h"
 #include "source/val/instruction.h"
+#include "source/val/validate_scopes.h"
 #include "source/val/validation_state.h"
 
 namespace spvtools {
 namespace val {
 namespace {
 
-spv_result_t ValidateExecutionScope(ValidationState_t& _,
-                                    const Instruction* inst, uint32_t scope) {
-  SpvOp opcode = inst->opcode();
-  bool is_int32 = false, is_const_int32 = false;
-  uint32_t value = 0;
-  std::tie(is_int32, is_const_int32, value) = _.EvalInt32IfConst(scope);
-
-  if (!is_int32) {
-    return _.diag(SPV_ERROR_INVALID_DATA, inst)
-           << spvOpcodeString(opcode)
-           << ": expected Execution Scope to be a 32-bit int";
-  }
-
-  if (!is_const_int32) {
-    return SPV_SUCCESS;
-  }
-
-  if (spvIsVulkanEnv(_.context()->target_env) &&
-      _.context()->target_env != SPV_ENV_VULKAN_1_0 &&
-      value != SpvScopeSubgroup) {
-    return _.diag(SPV_ERROR_INVALID_DATA, inst)
-           << spvOpcodeString(opcode)
-           << ": in Vulkan environment Execution scope is limited to "
-              "Subgroup";
-  }
-
-  if (value != SpvScopeSubgroup && value != SpvScopeWorkgroup) {
-    return _.diag(SPV_ERROR_INVALID_DATA, inst)
-           << spvOpcodeString(opcode)
-           << ": Execution scope is limited to Subgroup or Workgroup";
-  }
-
-  return SPV_SUCCESS;
-}
-
 spv_result_t ValidateGroupNonUniformBallotBitCount(ValidationState_t& _,
                                                    const Instruction* inst) {
   // Scope is already checked by ValidateExecutionScope() above.

source/val/validate_scopes.cpp

@@ -0,0 +1,104 @@
+// Copyright (c) 2018 Google LLC.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "source/val/validate_scopes.h"
+
+#include "source/diagnostic.h"
+#include "source/spirv_target_env.h"
+#include "source/val/instruction.h"
+#include "source/val/validation_state.h"
+
+namespace spvtools {
+namespace val {
+
+spv_result_t ValidateExecutionScope(ValidationState_t& _,
+                                    const Instruction* inst, uint32_t scope) {
+  SpvOp opcode = inst->opcode();
+  bool is_int32 = false, is_const_int32 = false;
+  uint32_t value = 0;
+  std::tie(is_int32, is_const_int32, value) = _.EvalInt32IfConst(scope);
+
+  if (!is_int32) {
+    return _.diag(SPV_ERROR_INVALID_DATA, inst)
+           << spvOpcodeString(opcode)
+           << ": expected Execution Scope to be a 32-bit int";
+  }
+
+  if (!is_const_int32) {
+    return SPV_SUCCESS;
+  }
+
+  // Vulkan specific rules
+  if (spvIsVulkanEnv(_.context()->target_env)) {
+    // Vulkan 1.1 specific rules
+    if (_.context()->target_env != SPV_ENV_VULKAN_1_0) {
+      // Scope for Non Uniform Group Operations must be limited to Subgroup
+      if (spvOpcodeIsNonUniformGroupOperation(opcode) &&
+          value != SpvScopeSubgroup) {
+        return _.diag(SPV_ERROR_INVALID_DATA, inst)
+               << spvOpcodeString(opcode)
+               << ": in Vulkan environment Execution scope is limited to "
+                  "Subgroup";
+      }
+    }
+
+    // If OpControlBarrier is used in fragment, vertex, tessellation evaluation,
+    // or geometry stages, the execution Scope must be Subgroup.
+    if (opcode == SpvOpControlBarrier && value != SpvScopeSubgroup) {
+      _.function(inst->function()->id())
+          ->RegisterExecutionModelLimitation([](SpvExecutionModel model,
+                                                std::string* message) {
+            if (model == SpvExecutionModelFragment ||
+                model == SpvExecutionModelVertex ||
+                model == SpvExecutionModelGeometry ||
+                model == SpvExecutionModelTessellationEvaluation) {
+              if (message) {
+                *message =
+                    "in Vulkan evironment, OpControlBarrier execution scope "
+                    "must be Subgroup for Fragment, Vertex, Geometry and "
+                    "TessellationEvaluation execution models";
+              }
+              return false;
+            }
+            return true;
+          });
+    }
+
+    // Vulkan generic rules
+    // Scope for execution must be limited to Workgroup or Subgroup
+    if (value != SpvScopeWorkgroup && value != SpvScopeSubgroup) {
+      return _.diag(SPV_ERROR_INVALID_DATA, inst)
+             << spvOpcodeString(opcode)
+             << ": in Vulkan environment Execution Scope is limited to "
+                "Workgroup and Subgroup";
+    }
+  }
+
+  // TODO(atgoo@github.com) Add checks for OpenCL and OpenGL environments.
+
+  // General SPIRV rules
+  // Scope for execution must be limited to Workgroup or Subgroup for
+  // non-uniform operations
+  if (spvOpcodeIsNonUniformGroupOperation(opcode) &&
+      value != SpvScopeSubgroup && value != SpvScopeWorkgroup) {
+    return _.diag(SPV_ERROR_INVALID_DATA, inst)
+           << spvOpcodeString(opcode)
+           << ": Execution scope is limited to Subgroup or Workgroup";
+  }
+
+  return SPV_SUCCESS;
+}
+
+}  // namespace val
+}  // namespace spvtools

source/val/validate_scopes.h

@@ -0,0 +1,27 @@
+// Copyright (c) 2018 Google LLC.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Validates correctness of scopes for SPIR-V instructions.
+
+#include "source/opcode.h"
+#include "source/val/validate.h"
+
+namespace spvtools {
+namespace val {
+
+spv_result_t ValidateExecutionScope(ValidationState_t& _,
+                                    const Instruction* inst, uint32_t scope);
+
+}  // namespace val
+}  // namespace spvtools