spirv-val: accept NonReadable/NonWritable on tensor variables in UniformConstant (KhronosGroup#6184)

* spirv-val: accept NonReadable/NonWritable on tensor variables in UniformConstant

Signed-off-by: Kevin Petit <[email protected]>
Change-Id: I9925002d576f767a833c303e8191b75aa349f0b8

* review comments

Change-Id: I7df3f9bfb130d9ddb964aa7637120a5eb07bd839

* deal with unused variable

Change-Id: I05844f8de07027429d97b837d9910e033b9739d9

* attempt to fix CI

Change-Id: I2b7f32a0b43b92071550fa05a580429e10d478a1

* attempt to fix CI

Change-Id: I10523fce45506590ced8dcd1d5662a83e207314b

* attempt to fix CI

Change-Id: Ieaffedea8d64f21c29e789e132b3b6533eccb678

* revert CI changes

---------

Signed-off-by: Kevin Petit <[email protected]>

Author: kpet

source/val/validate_decorations.cpp

@@ -1767,14 +1767,19 @@ spv_result_t CheckFPRoundingModeForShaders(ValidationState_t& vstate,
   return SPV_SUCCESS;
 }
 
-// Returns SPV_SUCCESS if validation rules are satisfied for the NonWritable
+// Returns SPV_SUCCESS if validation rules are satisfied for the NonReadable or
+// NonWritable
 // decoration.  Otherwise emits a diagnostic and returns something other than
 // SPV_SUCCESS.  The |inst| parameter is the object being decorated.  This must
 // be called after TypePass and AnnotateCheckDecorationsOfBuffers are called.
-spv_result_t CheckNonWritableDecoration(ValidationState_t& vstate,
-                                        const Instruction& inst,
-                                        const Decoration& decoration) {
+spv_result_t CheckNonReadableWritableDecorations(ValidationState_t& vstate,
+                                                 const Instruction& inst,
+                                                 const Decoration& decoration) {
   assert(inst.id() && "Parser ensures the target of the decoration has an ID");
+  const bool is_non_writable =
+      decoration.dec_type() == spv::Decoration::NonWritable;
+  assert(is_non_writable ||
+         decoration.dec_type() == spv::Decoration::NonReadable);
 
   if (decoration.struct_member_index() == Decoration::kInvalidMember) {
     // The target must be a memory object declaration.
@@ -1786,7 +1791,10 @@ spv_result_t CheckNonWritableDecoration(ValidationState_t& vstate,
         opcode != spv::Op::OpFunctionParameter &&
         opcode != spv::Op::OpRawAccessChainNV) {
       return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
-             << "Target of NonWritable decoration must be a memory object "
+             << "Target of "
+             << (is_non_writable ? "NonWritable" : "NonReadable")
+             << " decoration must be a "
+                "memory object "
                 "declaration (a variable or a function parameter)";
     }
     const auto var_storage_class =
@@ -1797,20 +1805,27 @@ spv_result_t CheckNonWritableDecoration(ValidationState_t& vstate,
                   : spv::StorageClass::Max;
     if ((var_storage_class == spv::StorageClass::Function ||
          var_storage_class == spv::StorageClass::Private) &&
-        vstate.features().nonwritable_var_in_function_or_private) {
+        vstate.features().nonwritable_var_in_function_or_private &&
+        is_non_writable) {
       // New permitted feature in SPIR-V 1.4.
     } else if (var_storage_class == spv::StorageClass::TileAttachmentQCOM) {
     } else if (
         // It may point to a UBO, SSBO, storage image, or raw access chain.
         vstate.IsPointerToUniformBlock(type_id) ||
         vstate.IsPointerToStorageBuffer(type_id) ||
         vstate.IsPointerToStorageImage(type_id) ||
+        vstate.IsPointerToTensor(type_id) ||
         opcode == spv::Op::OpRawAccessChainNV) {
     } else {
       return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
-             << "Target of NonWritable decoration is invalid: must point to a "
-                "storage image, uniform block, "
-             << (vstate.features().nonwritable_var_in_function_or_private
+             << "Target of "
+             << (is_non_writable ? "NonWritable" : "NonReadable")
+             << " decoration is invalid: "
+                "must point to a "
+                "storage image, tensor variable in UniformConstant storage "
+                "class, uniform block, "
+             << (vstate.features().nonwritable_var_in_function_or_private &&
+                         is_non_writable
                      ? "storage buffer, or variable in Private or Function "
                        "storage class"
                      : "or storage buffer");
@@ -2098,8 +2113,10 @@ spv_result_t CheckDecorationsFromDecoration(ValidationState_t& vstate) {
             PASS_OR_BAIL(
                 CheckFPRoundingModeForShaders(vstate, *inst, decoration));
           break;
+        case spv::Decoration::NonReadable:
         case spv::Decoration::NonWritable:
-          PASS_OR_BAIL(CheckNonWritableDecoration(vstate, *inst, decoration));
+          PASS_OR_BAIL(
+              CheckNonReadableWritableDecorations(vstate, *inst, decoration));
           break;
         case spv::Decoration::Uniform:
         case spv::Decoration::UniformId:

source/val/validate_type.cpp

@@ -562,6 +562,9 @@ spv_result_t ValidateTypePointer(ValidationState_t& _,
       // a storage image.
       if (sampled == 2) _.RegisterPointerToStorageImage(inst->id());
     }
+    if (type->opcode() == spv::Op::OpTypeTensorARM) {
+      _.RegisterPointerToTensor(inst->id());
+    }
   }
 
   if (!_.IsValidStorageClass(storage_class)) {

source/val/validation_state.h

@@ -772,6 +772,16 @@ class ValidationState_t {
     pointer_to_storage_image_.insert(type_id);
   }
 
+  // Is the ID the type of a pointer to a tensor?  That is, the pointee
+  // type is a tensor type.
+  bool IsPointerToTensor(uint32_t type_id) const {
+    return pointer_to_tensor_.find(type_id) != pointer_to_tensor_.cend();
+  }
+  // Save the ID of a pointer to a tensor.
+  void RegisterPointerToTensor(uint32_t type_id) {
+    pointer_to_tensor_.insert(type_id);
+  }
+
   // Tries to evaluate a any scalar integer OpConstant as uint64.
   // OpConstantNull is defined as zero for scalar int (will return true)
   // OpSpecConstant* return false since their values cannot be relied upon
@@ -1036,6 +1046,9 @@ class ValidationState_t {
   // The IDs of types of pointers to storage images.  This is populated in the
   // TypePass.
   std::unordered_set<uint32_t> pointer_to_storage_image_;
+  // The IDs of types of pointers to tensors.  This is populated in the
+  // TypePass.
+  std::unordered_set<uint32_t> pointer_to_tensor_;
 
   /// Maps ids to friendly names.
   std::unique_ptr<spvtools::FriendlyNameMapper> friendly_mapper_;