Merge two implementations of ValidateExecutionScope (#2131)
diff --git a/Android.mk b/Android.mk index bc781d5..07618c4 100644 --- a/Android.mk +++ b/Android.mk
@@ -68,6 +68,7 @@ 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 := \
diff --git a/BUILD.gn b/BUILD.gn index 529b844..bbce9f6 100644 --- a/BUILD.gn +++ b/BUILD.gn
@@ -424,6 +424,7 @@ "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", ]
diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt index 287c0df..da3c1cb 100644 --- a/source/CMakeLists.txt +++ b/source/CMakeLists.txt
@@ -319,6 +319,7 @@ ${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
diff --git a/source/val/validate_barriers.cpp b/source/val/validate_barriers.cpp index aa756ed..3ed0f57 100644 --- a/source/val/validate_barriers.cpp +++ b/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) {
diff --git a/source/val/validate_non_uniform.cpp b/source/val/validate_non_uniform.cpp index 94c7c2c..8dcf974 100644 --- a/source/val/validate_non_uniform.cpp +++ b/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.
diff --git a/source/val/validate_scopes.cpp b/source/val/validate_scopes.cpp new file mode 100644 index 0000000..a5ac7ae --- /dev/null +++ b/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
diff --git a/source/val/validate_scopes.h b/source/val/validate_scopes.h new file mode 100644 index 0000000..40e533e --- /dev/null +++ b/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