spirv-val: Add Vulkan Location VUID

KhronosGroup · Nov 1, 2021 · afbea88 · afbea88
1 parent 791f5b4
commit afbea88
Show file tree

Hide file tree

Showing 9 changed files with 347 additions and 20 deletions.
diff --git a/source/val/validate.cpp b/source/val/validate.cpp
@@ -227,8 +227,11 @@ spv_result_t ValidateBinaryUsingContextAndValidationState(
         desc.name = desc_name;
 
         std::vector<uint32_t> interfaces;
-        for (size_t j = 3; j < inst->operands().size(); ++j)
-          desc.interfaces.push_back(inst->word(inst->operand(j).offset));
+        for (size_t j = 3; j < inst->operands().size(); ++j) {
+          const auto id = inst->word(inst->operand(j).offset);
+          desc.interfaces.push_back(id);
+          vstate->registerInterfaceVariable(id);
+        }
 
         vstate->RegisterEntryPoint(entry_point, execution_model,
                                    std::move(desc));

diff --git a/source/val/validate_decorations.cpp b/source/val/validate_decorations.cpp
@@ -702,7 +702,7 @@ spv_result_t CheckBuiltInVariable(uint32_t var_id, ValidationState_t& vstate) {
       if (d.dec_type() == SpvDecorationLocation ||
           d.dec_type() == SpvDecorationComponent) {
         return vstate.diag(SPV_ERROR_INVALID_ID, vstate.FindDef(var_id))
-               << "A BuiltIn variable (id " << var_id
+               << vstate.VkErrorID(4915) << "A BuiltIn variable (id " << var_id
                << ") cannot have any Location or Component decorations";
       }
     }
@@ -1621,16 +1621,41 @@ spv_result_t CheckBlockDecoration(ValidationState_t& vstate,
 spv_result_t CheckLocationDecoration(ValidationState_t& vstate,
                                      const Instruction& inst,
                                      const Decoration& decoration) {
-  if (inst.opcode() == SpvOpVariable) return SPV_SUCCESS;
+  if (inst.opcode() != SpvOpVariable &&
+      (inst.opcode() != SpvOpTypeStruct ||
+       decoration.struct_member_index() == Decoration::kInvalidMember)) {
+    return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
+           << "Location decoration can only be applied to a variable or member "
+              "of a structure type";
+  }
+
+  if (spvIsVulkanEnv(vstate.context()->target_env)) {
+    bool found = false;
+    for (auto interface_var_id : vstate.interface_vars()) {
+      uint32_t var_struct_id = 0;
+      // for struct member need to see if interface variable is pointing to
+      // struct
+      if (inst.opcode() == SpvOpTypeStruct) {
+        auto var = vstate.FindDef(interface_var_id);
+        auto var_ptr = vstate.FindDef(var->GetOperandAs<uint32_t>(0));
+        var_struct_id =
+            vstate.FindDef(var_ptr->GetOperandAs<uint32_t>(2))->id();
+      }
+
+      if ((var_struct_id == inst.id() || interface_var_id == inst.id())) {
+        found = true;
+        break;
+      }
+    }
 
-  if (decoration.struct_member_index() != Decoration::kInvalidMember &&
-      inst.opcode() == SpvOpTypeStruct) {
-    return SPV_SUCCESS;
+    if (!found) {
+      return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
+             << vstate.VkErrorID(4916)
+             << "Location decorations must be used on user-defined variables.";
+    }
   }
 
-  return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
-         << "Location decoration can only be applied to a variable or member "
-            "of a structure type";
+  return SPV_SUCCESS;
 }
 
 #define PASS_OR_BAIL_AT_LINE(X, LINE)           \

diff --git a/source/val/validate_interfaces.cpp b/source/val/validate_interfaces.cpp
@@ -154,7 +154,7 @@ spv_result_t NumConsumedLocations(ValidationState_t& _, const Instruction* type,
       // Members cannot have location decorations at this point.
       if (_.HasDecoration(type->id(), SpvDecorationLocation)) {
         return _.diag(SPV_ERROR_INVALID_DATA, type)
-               << "Members cannot be assigned a location";
+               << _.VkErrorID(4918) << "Members cannot be assigned a location";
       }
 
       // Structs consume locations equal to the sum of the locations consumed
@@ -327,7 +327,7 @@ spv_result_t GetLocationsForVariable(
   const bool is_block = _.HasDecoration(type_id, SpvDecorationBlock);
   if (!has_location && !is_block) {
     return _.diag(SPV_ERROR_INVALID_DATA, variable)
-           << "Variable must be decorated with a location";
+           << _.VkErrorID(4917) << "Variable must be decorated with a location";
   }
 
   const std::string storage_class = is_output ? "output" : "input";
@@ -411,7 +411,7 @@ spv_result_t GetLocationsForVariable(
       auto where = member_locations.find(i - 1);
       if (where == member_locations.end()) {
         return _.diag(SPV_ERROR_INVALID_DATA, type)
-               << "Member index " << i - 1
+               << _.VkErrorID(4919) << "Member index " << i - 1
                << " is missing a location assignment";
       }
 

diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp
@@ -153,6 +153,7 @@ ValidationState_t::ValidationState_t(const spv_const_context ctx,
       module_extensions_(),
       ordered_instructions_(),
       all_definitions_(),
+      interface_vars_(),
       global_vars_(),
       local_vars_(),
       struct_nesting_depth_(),
@@ -1865,6 +1866,16 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
       return VUID_WRAP(VUID-StandaloneSpirv-OpMemoryBarrier-04733);
     case 4780:
       return VUID_WRAP(VUID-StandaloneSpirv-Result-04780);
+    case 4915:
+      return VUID_WRAP(VUID-StandaloneSpirv-Location-04915);
+    case 4916:
+      return VUID_WRAP(VUID-StandaloneSpirv-Location-04916);
+    case 4917:
+      return VUID_WRAP(VUID-StandaloneSpirv-Location-04917);
+    case 4918:
+      return VUID_WRAP(VUID-StandaloneSpirv-Location-04918);
+    case 4919:
+      return VUID_WRAP(VUID-StandaloneSpirv-Location-04919);
     default:
       return "";  // unknown id
   }

diff --git a/source/val/validation_state.h b/source/val/validation_state.h
@@ -455,18 +455,29 @@ class ValidationState_t {
   void RegisterStorageClassConsumer(SpvStorageClass storage_class,
                                     Instruction* consumer);
 
+  /// Returns the set of Interface Variables.
+  std::unordered_set<uint32_t>& interface_vars() { return interface_vars_; }
+
   /// Returns the set of Global Variables.
   std::unordered_set<uint32_t>& global_vars() { return global_vars_; }
 
   /// Returns the set of Local Variables.
   std::unordered_set<uint32_t>& local_vars() { return local_vars_; }
 
+  /// Returns the number of Interface Variables.
+  size_t num_interface_vars() { return interface_vars_.size(); }
+
   /// Returns the number of Global Variables.
   size_t num_global_vars() { return global_vars_.size(); }
 
   /// Returns the number of Local Variables.
   size_t num_local_vars() { return local_vars_.size(); }
 
+  /// Inserts a new <id> to the set of Interface Variables.
+  void registerInterfaceVariable(const uint32_t id) {
+    interface_vars_.insert(id);
+  }
+
   /// Inserts a new <id> to the set of Global Variables.
   void registerGlobalVariable(const uint32_t id) { global_vars_.insert(id); }
 
@@ -811,6 +822,9 @@ class ValidationState_t {
   /// ID Bound from the Header
   uint32_t id_bound_;
 
+  /// Set of Interface Variable IDs (collection of all entry points)
+  std::unordered_set<uint32_t> interface_vars_;
+
   /// Set of Global Variable IDs (Storage Class other than 'Function')
   std::unordered_set<uint32_t> global_vars_;