Split mode setting opcode validation into new file.

* Moved mode setting opcode validation out of idUsage and into a new
pass
 * minor style updates

Android.mk

@@ -56,6 +56,7 @@ SPVTOOLS_SRC_FILES := \
 		source/val/validate_interfaces.cpp \
 		source/val/validate_instruction.cpp \
 		source/val/validate_memory.cpp \
+		source/val/validate_mode_setting.cpp \
 		source/val/validate_layout.cpp \
 		source/val/validate_literals.cpp \
 		source/val/validate_logicals.cpp \

BUILD.gn

@@ -382,6 +382,7 @@ static_library("spvtools_val") {
     "source/val/validate_literals.cpp",
     "source/val/validate_logicals.cpp",
     "source/val/validate_memory.cpp",
+    "source/val/validate_mode_setting.cpp",
     "source/val/validate_non_uniform.cpp",
     "source/val/validate_primitives.cpp",
     "source/val/validate_type.cpp",

source/CMakeLists.txt

@@ -306,6 +306,7 @@ set(SPIRV_SOURCES
   ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_literals.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_logicals.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_memory.cpp
+  ${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_type.cpp

source/val/validate.cpp

@@ -324,7 +324,7 @@ spv_result_t ValidateBinaryUsingContextAndValidationState(
     if (auto error = DebugPass(*vstate, &instruction)) return error;
     if (auto error = AnnotationPass(*vstate, &instruction)) return error;
     if (auto error = ExtInstPass(*vstate, &instruction)) return error;
-    // Mode Setting
+    if (auto error = ModeSettingPass(*vstate, &instruction)) return error;
     if (auto error = TypePass(*vstate, &instruction)) return error;
     // Constants
     if (auto error = ValidateMemoryInstructions(*vstate, &instruction))

source/val/validate.h

@@ -188,6 +188,9 @@ spv_result_t CapabilityPass(ValidationState_t& _, const Instruction* inst);
 /// Validates correctness of primitive instructions.
 spv_result_t PrimitivesPass(ValidationState_t& _, const Instruction* inst);
 
+/// Validates correctness of mode setting instructions.
+spv_result_t ModeSettingPass(ValidationState_t& _, const Instruction* inst);
+
 /// @brief Validate the ID usage of the instruction stream
 ///
 /// @param[in] pInsts stream of instructions

source/val/validate_id.cpp

@@ -91,59 +91,6 @@ class idUsage {
                           SPV_ERROR_INVALID_DIAGNOSTIC);                    \
   helper
 
-template <>
-bool idUsage::isValid<SpvOpEntryPoint>(const spv_instruction_t* inst,
-                                       const spv_opcode_desc) {
-  auto entryPointIndex = 2;
-  auto entryPoint = module_.FindDef(inst->words[entryPointIndex]);
-  if (!entryPoint || SpvOpFunction != entryPoint->opcode()) {
-    DIAG(entryPoint) << "OpEntryPoint Entry Point <id> '"
-                     << module_.getIdName(inst->words[entryPointIndex])
-                     << "' is not a function.";
-    return false;
-  }
-  // don't check kernel function signatures
-  const SpvExecutionModel executionModel = SpvExecutionModel(inst->words[1]);
-  if (executionModel != SpvExecutionModelKernel) {
-    // TODO: Check the entry point signature is void main(void), may be subject
-    // to change
-    auto entryPointType = module_.FindDef(entryPoint->words()[4]);
-    if (!entryPointType || 3 != entryPointType->words().size()) {
-      DIAG(entryPoint) << "OpEntryPoint Entry Point <id> '"
-                       << module_.getIdName(inst->words[entryPointIndex])
-                       << "'s function parameter count is not zero.";
-      return false;
-    }
-  }
-
-  auto returnType = module_.FindDef(entryPoint->type_id());
-  if (!returnType || SpvOpTypeVoid != returnType->opcode()) {
-    DIAG(entryPoint) << "OpEntryPoint Entry Point <id> '"
-                     << module_.getIdName(inst->words[entryPointIndex])
-                     << "'s function return type is not void.";
-    return false;
-  }
-  return true;
-}
-
-template <>
-bool idUsage::isValid<SpvOpExecutionMode>(const spv_instruction_t* inst,
-                                          const spv_opcode_desc) {
-  auto entryPointIndex = 1;
-  auto entryPointID = inst->words[entryPointIndex];
-  auto found =
-      std::find(entry_points_.cbegin(), entry_points_.cend(), entryPointID);
-  if (found == entry_points_.cend()) {
-    DIAG(module_.FindDef(entryPointID))
-        << "OpExecutionMode Entry Point <id> '"
-        << module_.getIdName(inst->words[entryPointIndex])
-        << "' is not the Entry Point "
-           "operand of an OpEntryPoint.";
-    return false;
-  }
-  return true;
-}
-
 template <>
 bool idUsage::isValid<SpvOpConstantTrue>(const spv_instruction_t* inst,
                                          const spv_opcode_desc) {
@@ -1024,8 +971,6 @@ bool idUsage::isValid(const spv_instruction_t* inst) {
   case Spv##OpCode:  \
     return isValid<Spv##OpCode>(inst, opcodeEntry);
   switch (inst->opcode) {
-    CASE(OpEntryPoint)
-    CASE(OpExecutionMode)
     CASE(OpConstantTrue)
     CASE(OpConstantFalse)
     CASE(OpConstantComposite)

source/val/validate_mode_setting.cpp

@@ -0,0 +1,91 @@
+// 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.h"
+
+#include <algorithm>
+
+#include "source/opcode.h"
+#include "source/val/instruction.h"
+#include "source/val/validation_state.h"
+
+namespace spvtools {
+namespace val {
+namespace {
+
+spv_result_t ValidateEntryPoint(ValidationState_t& _, const Instruction* inst) {
+  const auto entry_point_id = inst->GetOperandAs<uint32_t>(1);
+  auto entry_point = _.FindDef(entry_point_id);
+  if (!entry_point || SpvOpFunction != entry_point->opcode()) {
+    return _.diag(SPV_ERROR_INVALID_ID, inst)
+           << "OpEntryPoint Entry Point <id> '" << _.getIdName(entry_point_id)
+           << "' is not a function.";
+  }
+  // don't check kernel function signatures
+  const SpvExecutionModel execution_model =
+      inst->GetOperandAs<SpvExecutionModel>(0);
+  if (execution_model != SpvExecutionModelKernel) {
+    // TODO: Check the entry point signature is void main(void), may be subject
+    // to change
+    const auto entry_point_type_id = entry_point->GetOperandAs<uint32_t>(3);
+    const auto entry_point_type = _.FindDef(entry_point_type_id);
+    if (!entry_point_type || 3 != entry_point_type->words().size()) {
+      return _.diag(SPV_ERROR_INVALID_ID, inst)
+             << "OpEntryPoint Entry Point <id> '" << _.getIdName(entry_point_id)
+             << "'s function parameter count is not zero.";
+    }
+  }
+
+  auto return_type = _.FindDef(entry_point->type_id());
+  if (!return_type || SpvOpTypeVoid != return_type->opcode()) {
+    return _.diag(SPV_ERROR_INVALID_ID, inst)
+           << "OpEntryPoint Entry Point <id> '" << _.getIdName(entry_point_id)
+           << "'s function return type is not void.";
+  }
+  return SPV_SUCCESS;
+}
+
+spv_result_t ValidateExecutionMode(ValidationState_t& _,
+                                   const Instruction* inst) {
+  const auto entry_point_id = inst->GetOperandAs<uint32_t>(0);
+  const auto found = std::find(_.entry_points().cbegin(),
+                               _.entry_points().cend(), entry_point_id);
+  if (found == _.entry_points().cend()) {
+    return _.diag(SPV_ERROR_INVALID_ID, inst)
+           << "OpExecutionMode Entry Point <id> '"
+           << _.getIdName(entry_point_id)
+           << "' is not the Entry Point "
+              "operand of an OpEntryPoint.";
+  }
+  return SPV_SUCCESS;
+}
+
+}  // namespace
+
+spv_result_t ModeSettingPass(ValidationState_t& _, const Instruction* inst) {
+  switch (inst->opcode()) {
+    case SpvOpEntryPoint:
+      if (auto error = ValidateEntryPoint(_, inst)) return error;
+      break;
+    case SpvOpExecutionMode:
+      if (auto error = ValidateExecutionMode(_, inst)) return error;
+      break;
+    default:
+      break;
+  }
+  return SPV_SUCCESS;
+}
+
+}  // namespace val
+}  // namespace spvtools