Fixes #1483. Validating Vulkan 1.1 barrier execution scopes

* Reworked how execution model limitations are checked
 * Now OpFunction checks which entry points call it and checks its
 registered limitations instead of building a call stack in the entry
 point
* New tests
* Moving function to entry point mapping into VState

source/val/validation_state.cpp

@@ -15,6 +15,7 @@
 #include "val/validation_state.h"
 
 #include <cassert>
+#include <stack>
 
 #include "opcode.h"
 #include "val/basic_block.h"
@@ -792,4 +793,37 @@ std::tuple<bool, bool, uint32_t> ValidationState_t::EvalInt32IfConst(
   return std::make_tuple(true, true, inst->word(3));
 }
 
+void ValidationState_t::ComputeFunctionToEntryPointMapping() {
+  for (const uint32_t entry_point : entry_points()) {
+    std::stack<uint32_t> call_stack;
+    std::set<uint32_t> visited;
+    call_stack.push(entry_point);
+    while (!call_stack.empty()) {
+      const uint32_t called_func_id = call_stack.top();
+      call_stack.pop();
+      if (!visited.insert(called_func_id).second) continue;
+
+      function_to_entry_points_[called_func_id].push_back(entry_point);
+
+      const Function* called_func = function(called_func_id);
+      if (called_func) {
+        // Other checks should error out on this invalid SPIR-V.
+        for (const uint32_t new_call : called_func->function_call_targets()) {
+          call_stack.push(new_call);
+        }
+      }
+    }
+  }
+}
+
+const std::vector<uint32_t>& ValidationState_t::FunctionEntryPoints(
+    uint32_t func) const {
+  auto iter = function_to_entry_points_.find(func);
+  if (iter == function_to_entry_points_.end()) {
+    return empty_ids_;
+  } else {
+    return iter->second;
+  }
+}
+
 }  // namespace libspirv

source/val/validation_state.h

@@ -208,6 +208,13 @@ class ValidationState_t {
     return &it->second;
   }
 
+  /// Traverses call tree and computes function_to_entry_points_.
+  /// Note: called after fully parsing the binary.
+  void ComputeFunctionToEntryPointMapping();
+
+  /// Returns all the entry points that can call |func|.
+  const std::vector<uint32_t>& FunctionEntryPoints(uint32_t func) const;
+
   /// Inserts an <id> to the set of functions that are target of OpFunctionCall.
   void AddFunctionCallTarget(const uint32_t id) {
     function_call_targets_.insert(id);
@@ -559,6 +566,11 @@ class ValidationState_t {
   /// Mapping entry point -> execution modes.
   std::unordered_map<uint32_t, std::set<SpvExecutionMode>>
       entry_point_to_execution_modes_;
+
+  /// Mapping function -> array of entry points inside this
+  /// module which can (indirectly) call the function.
+  std::unordered_map<uint32_t, std::vector<uint32_t>> function_to_entry_points_;
+  const std::vector<uint32_t> empty_ids_;
 };
 
 }  // namespace libspirv

source/validate.cpp

@@ -298,6 +298,8 @@ spv_result_t ValidateBinaryUsingContextAndValidationState(
            << id_str.substr(0, id_str.size() - 1);
   }
 
+  vstate->ComputeFunctionToEntryPointMapping();
+
   // Validate the preconditions involving adjacent instructions. e.g. SpvOpPhi
   // must only be preceeded by SpvOpLabel, SpvOpPhi, or SpvOpLine.
   if (auto error = ValidateAdjacency(*vstate)) return error;

source/validate_barriers.cpp

@@ -54,6 +54,26 @@ spv_result_t ValidateExecutionScope(ValidationState_t& _,
              << ": in Vulkan environment Execution Scope is limited to "
                 "Workgroup and Subgroup";
     }
+
+    if (_.context()->target_env != SPV_ENV_VULKAN_1_0 &&
+        value != SpvScopeSubgroup) {
+      _.current_function().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.

source/validate_builtins.cpp

@@ -374,9 +374,6 @@ class BuiltInsValidator {
   // instruction.
   void Update(const Instruction& inst);
 
-  // Traverses call tree and computes function_to_entry_points_.
-  void ComputeFunctionToEntryPointMapping();
-
   const ValidationState_t& _;
 
   // Mapping id -> list of rules which validate instruction referencing the
@@ -395,10 +392,6 @@ class BuiltInsValidator {
   const std::vector<uint32_t> no_entry_points;
   const std::vector<uint32_t>* entry_points_ = &no_entry_points;
 
-  // Mapping function -> array of entry points inside this
-  // module which can (indirectly) call the function.
-  std::unordered_map<uint32_t, std::vector<uint32_t>> function_to_entry_points_;
-
   // Execution models with which the current function can be called.
   std::set<SpvExecutionModel> execution_models_;
 };
@@ -410,17 +403,12 @@ void BuiltInsValidator::Update(const Instruction& inst) {
     assert(function_id_ == 0);
     function_id_ = inst.id();
     execution_models_.clear();
-    const auto it = function_to_entry_points_.find(function_id_);
+    entry_points_ = &_.FunctionEntryPoints(function_id_);
-    if (it == function_to_entry_points_.end()) {
+    // Collect execution models from all entry points from which the current
-      entry_points_ = &no_entry_points;
+    // function can be called.
-    } else {
+    for (const uint32_t entry_point : *entry_points_) {
-      entry_points_ = &it->second;
+      if (const auto* models = _.GetExecutionModels(entry_point)) {
-      // Collect execution models from all entry points from which the current
+        execution_models_.insert(models->begin(), models->end());
-      // function can be called.
-      for (const uint32_t entry_point : *entry_points_) {
-        if (const auto* models = _.GetExecutionModels(entry_point)) {
-          execution_models_.insert(models->begin(), models->end());
-        }
       }
     }
   }
@@ -434,28 +422,6 @@ void BuiltInsValidator::Update(const Instruction& inst) {
   }
 }
 
-void BuiltInsValidator::ComputeFunctionToEntryPointMapping() {
-  // TODO: Move this into validation_state.cpp.
-  for (const uint32_t entry_point : _.entry_points()) {
-    std::stack<uint32_t> call_stack;
-    std::set<uint32_t> visited;
-    call_stack.push(entry_point);
-    while (!call_stack.empty()) {
-      const uint32_t called_func_id = call_stack.top();
-      call_stack.pop();
-      if (!visited.insert(called_func_id).second) continue;
-
-      function_to_entry_points_[called_func_id].push_back(entry_point);
-
-      const Function* called_func = _.function(called_func_id);
-      assert(called_func);
-      for (const uint32_t new_call : called_func->function_call_targets()) {
-        call_stack.push(new_call);
-      }
-    }
-  }
-}
-
 std::string BuiltInsValidator::GetDefinitionDesc(
     const Decoration& decoration, const Instruction& inst) const {
   std::ostringstream ss;
@@ -2514,8 +2480,6 @@ spv_result_t BuiltInsValidator::Run() {
     return SPV_SUCCESS;
   }
 
-  ComputeFunctionToEntryPointMapping();
-
   // Second pass: validate every id reference in the module using
   // rules in id_to_at_reference_checks_.
   for (const Instruction& inst : _.ordered_instructions()) {

source/validate_id.cpp

@@ -303,32 +303,6 @@ bool idUsage::isValid<SpvOpEntryPoint>(const spv_instruction_t* inst,
     }
   }
 
-  std::stack<uint32_t> call_stack;
-  std::set<uint32_t> visited;
-  call_stack.push(entryPoint->id());
-  while (!call_stack.empty()) {
-    const uint32_t called_func_id = call_stack.top();
-    call_stack.pop();
-    if (!visited.insert(called_func_id).second) continue;
-
-    const Function* called_func = module_.function(called_func_id);
-    assert(called_func);
-
-    std::string reason;
-    if (!called_func->IsCompatibleWithExecutionModel(executionModel, &reason)) {
-      DIAG(entryPointIndex)
-          << "OpEntryPoint Entry Point <id> '" << inst->words[entryPointIndex]
-          << "'s callgraph contains function <id> " << called_func_id
-          << ", which cannot be used with the current execution model:\n"
-          << reason;
-      return false;
-    }
-
-    for (uint32_t new_call : called_func->function_call_targets()) {
-      call_stack.push(new_call);
-    }
-  }
-
   auto returnType = module_.FindDef(entryPoint->type_id());
   if (!returnType || SpvOpTypeVoid != returnType->opcode()) {
     DIAG(entryPointIndex) << "OpEntryPoint Entry Point <id> '"
@@ -1594,6 +1568,29 @@ bool idUsage::isValid<SpvOpGenericPtrMemSemantics>(
 template <>
 bool idUsage::isValid<SpvOpFunction>(const spv_instruction_t* inst,
                                      const spv_opcode_desc) {
+  const auto* thisInst = module_.FindDef(inst->words[2u]);
+  if (!thisInst) return false;
+
+  for (uint32_t entryId : module_.FunctionEntryPoints(thisInst->id())) {
+    const Function* thisFunc = module_.function(thisInst->id());
+    assert(thisFunc);
+    const auto* models = module_.GetExecutionModels(entryId);
+    if (models) {
+      assert(models->size());
+      for (auto model : *models) {
+        std::string reason;
+        if (!thisFunc->IsCompatibleWithExecutionModel(model, &reason)) {
+          DIAG(2)
+              << "OpEntryPoint Entry Point <id> '" << entryId
+              << "'s callgraph contains function <id> " << thisInst->id()
+              << ", which cannot be used with the current execution model:\n"
+              << reason;
+          return false;
+        }
+      }
+    }
+  }
+
   auto resultTypeIndex = 1;
   auto resultType = module_.FindDef(inst->words[resultTypeIndex]);
   if (!resultType) return false;

test/val/val_barriers_test.cpp

@@ -375,6 +375,167 @@ OpControlBarrier %workgroup %device %acquire_release_subgroup
           "Vulkan-supported storage class if Memory Semantics is not None"));
 }
 
+TEST_F(ValidateBarriers, OpControlBarrierSubgroupExecutionFragment1p1) {
+  const std::string body = R"(
+OpControlBarrier %subgroup %subgroup %acquire_release_subgroup
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body, "", "Fragment"),
+                      SPV_ENV_VULKAN_1_1);
+  ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_1));
+}
+
+TEST_F(ValidateBarriers, OpControlBarrierWorkgroupExecutionFragment1p1) {
+  const std::string body = R"(
+OpControlBarrier %workgroup %workgroup %acquire_release
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body, "", "Fragment"),
+                      SPV_ENV_VULKAN_1_1);
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpControlBarrier execution scope must be Subgroup for "
+                        "Fragment, Vertex, Geometry and TessellationEvaluation "
+                        "execution models"));
+}
+
+TEST_F(ValidateBarriers, OpControlBarrierSubgroupExecutionFragment1p0) {
+  const std::string body = R"(
+OpControlBarrier %subgroup %workgroup %acquire_release
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body, "", "Fragment"),
+                      SPV_ENV_VULKAN_1_0);
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_0));
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr("OpControlBarrier requires one of the following Execution "
+                "Models: TessellationControl, GLCompute or Kernel"));
+}
+
+TEST_F(ValidateBarriers, OpControlBarrierSubgroupExecutionVertex1p1) {
+  const std::string body = R"(
+OpControlBarrier %subgroup %subgroup %acquire_release_subgroup
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body, "", "Vertex"),
+                      SPV_ENV_VULKAN_1_1);
+  ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_1));
+}
+
+TEST_F(ValidateBarriers, OpControlBarrierWorkgroupExecutionVertex1p1) {
+  const std::string body = R"(
+OpControlBarrier %workgroup %workgroup %acquire_release
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body, "", "Vertex"),
+                      SPV_ENV_VULKAN_1_1);
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpControlBarrier execution scope must be Subgroup for "
+                        "Fragment, Vertex, Geometry and TessellationEvaluation "
+                        "execution models"));
+}
+
+TEST_F(ValidateBarriers, OpControlBarrierSubgroupExecutionVertex1p0) {
+  const std::string body = R"(
+OpControlBarrier %subgroup %workgroup %acquire_release
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body, "", "Vertex"),
+                      SPV_ENV_VULKAN_1_0);
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_0));
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr("OpControlBarrier requires one of the following Execution "
+                "Models: TessellationControl, GLCompute or Kernel"));
+}
+
+TEST_F(ValidateBarriers, OpControlBarrierSubgroupExecutionGeometry1p1) {
+  const std::string body = R"(
+OpControlBarrier %subgroup %subgroup %acquire_release_subgroup
+)";
+
+  CompileSuccessfully(
+      GenerateShaderCode(body, "OpCapability Geometry\n", "Geometry"),
+      SPV_ENV_VULKAN_1_1);
+  ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_1));
+}
+
+TEST_F(ValidateBarriers, OpControlBarrierWorkgroupExecutionGeometry1p1) {
+  const std::string body = R"(
+OpControlBarrier %workgroup %workgroup %acquire_release
+)";
+
+  CompileSuccessfully(
+      GenerateShaderCode(body, "OpCapability Geometry\n", "Geometry"),
+      SPV_ENV_VULKAN_1_1);
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpControlBarrier execution scope must be Subgroup for "
+                        "Fragment, Vertex, Geometry and TessellationEvaluation "
+                        "execution models"));
+}
+
+TEST_F(ValidateBarriers, OpControlBarrierSubgroupExecutionGeometry1p0) {
+  const std::string body = R"(
+OpControlBarrier %subgroup %workgroup %acquire_release
+)";
+
+  CompileSuccessfully(
+      GenerateShaderCode(body, "OpCapability Geometry\n", "Geometry"),
+      SPV_ENV_VULKAN_1_0);
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_0));
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr("OpControlBarrier requires one of the following Execution "
+                "Models: TessellationControl, GLCompute or Kernel"));
+}
+
+TEST_F(ValidateBarriers,
+       OpControlBarrierSubgroupExecutionTessellationEvaluation1p1) {
+  const std::string body = R"(
+OpControlBarrier %subgroup %subgroup %acquire_release_subgroup
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body, "OpCapability Tessellation\n",
+                                         "TessellationEvaluation"),
+                      SPV_ENV_VULKAN_1_1);
+  ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_1));
+}
+
+TEST_F(ValidateBarriers,
+       OpControlBarrierWorkgroupExecutionTessellationEvaluation1p1) {
+  const std::string body = R"(
+OpControlBarrier %workgroup %workgroup %acquire_release
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body, "OpCapability Tessellation\n",
+                                         "TessellationEvaluation"),
+                      SPV_ENV_VULKAN_1_1);
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpControlBarrier execution scope must be Subgroup for "
+                        "Fragment, Vertex, Geometry and TessellationEvaluation "
+                        "execution models"));
+}
+
+TEST_F(ValidateBarriers,
+       OpControlBarrierSubgroupExecutionTessellationEvaluation1p0) {
+  const std::string body = R"(
+OpControlBarrier %subgroup %workgroup %acquire_release
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body, "OpCapability Tessellation\n",
+                                         "TessellationEvaluation"),
+                      SPV_ENV_VULKAN_1_0);
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_0));
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr("OpControlBarrier requires one of the following Execution "
+                "Models: TessellationControl, GLCompute or Kernel"));
+}
+
 TEST_F(ValidateBarriers, OpMemoryBarrierSuccess) {
   const std::string body = R"(
 OpMemoryBarrier %cross_device %acquire_release_uniform_workgroup