Allow fmt arg to printf to be an array of i8 in non-constant space (KhronosGroup#5677)

* In spirv-val allow format arg to printf to be an array of i8 in Generic space

Signed-off-by: Lu, John <[email protected]>

* Allow more addr spaces for printf format string

Signed-off-by: Lu, John <[email protected]>

* Update printf format arg testcase

Signed-off-by: Lu, John <[email protected]>

* Apply clang-format

Signed-off-by: Lu, John <[email protected]>

* Reorder code for clarity

Signed-off-by: Lu, John <[email protected]>

* Only allow other addr spaces if extension is seen

Signed-off-by: Lu, John <[email protected]>

* Add test to check printf format with extension

Signed-off-by: Lu, John <[email protected]>

* Add extension correctly

Signed-off-by: Lu, John <[email protected]>

---------

Signed-off-by: Lu, John <[email protected]>

Author: LU-JOHN

source/val/validate_extensions.cpp

@@ -2962,12 +2962,38 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
                  << "expected operand Format to be a pointer";
         }
 
-        if (format_storage_class != spv::StorageClass::UniformConstant) {
-          return _.diag(SPV_ERROR_INVALID_DATA, inst)
-                 << ext_inst_name() << ": "
-                 << "expected Format storage class to be UniformConstant";
+        if (_.HasExtension(
+                Extension::kSPV_EXT_relaxed_printf_string_address_space)) {
+          if (format_storage_class != spv::StorageClass::UniformConstant &&
+              // Extension SPV_EXT_relaxed_printf_string_address_space allows
+              // format strings in Global, Local, Private and Generic address
+              // spaces
+
+              // Global
+              format_storage_class != spv::StorageClass::CrossWorkgroup &&
+              // Local
+              format_storage_class != spv::StorageClass::Workgroup &&
+              // Private
+              format_storage_class != spv::StorageClass::Function &&
+              // Generic
+              format_storage_class != spv::StorageClass::Generic) {
+            return _.diag(SPV_ERROR_INVALID_DATA, inst)
+                   << ext_inst_name() << ": "
+                   << "expected Format storage class to be UniformConstant, "
+                      "Crossworkgroup, Workgroup, Function, or Generic";
+          }
+        } else {
+          if (format_storage_class != spv::StorageClass::UniformConstant) {
+            return _.diag(SPV_ERROR_INVALID_DATA, inst)
+                   << ext_inst_name() << ": "
+                   << "expected Format storage class to be UniformConstant";
+          }
         }
 
+        // If pointer points to an array, get the type of an element
+        if (_.IsIntArrayType(format_data_type))
+          format_data_type = _.GetComponentType(format_data_type);
+
         if (!_.IsIntScalarType(format_data_type) ||
             _.GetBitWidth(format_data_type) != 8) {
           return _.diag(SPV_ERROR_INVALID_DATA, inst)

source/val/validation_state.cpp

@@ -869,6 +869,9 @@ uint32_t ValidationState_t::GetComponentType(uint32_t id) const {
     case spv::Op::OpTypeBool:
       return id;
 
+    case spv::Op::OpTypeArray:
+      return inst->word(2);
+
     case spv::Op::OpTypeVector:
       return inst->word(2);
 
@@ -992,6 +995,19 @@ bool ValidationState_t::IsIntScalarType(uint32_t id) const {
   return inst && inst->opcode() == spv::Op::OpTypeInt;
 }
 
+bool ValidationState_t::IsIntArrayType(uint32_t id) const {
+  const Instruction* inst = FindDef(id);
+  if (!inst) {
+    return false;
+  }
+
+  if (inst->opcode() == spv::Op::OpTypeArray) {
+    return IsIntScalarType(GetComponentType(id));
+  }
+
+  return false;
+}
+
 bool ValidationState_t::IsIntVectorType(uint32_t id) const {
   const Instruction* inst = FindDef(id);
   if (!inst) {

source/val/validation_state.h

@@ -606,6 +606,7 @@ class ValidationState_t {
   bool IsFloatScalarOrVectorType(uint32_t id) const;
   bool IsFloatMatrixType(uint32_t id) const;
   bool IsIntScalarType(uint32_t id) const;
+  bool IsIntArrayType(uint32_t id) const;
   bool IsIntVectorType(uint32_t id) const;
   bool IsIntScalarOrVectorType(uint32_t id) const;
   bool IsUnsignedIntScalarType(uint32_t id) const;

test/val/val_ext_inst_test.cpp

@@ -447,6 +447,7 @@ OpCapability Matrix
 %u8arr_uniform_constant = OpVariable %u8arr_ptr_uniform_constant UniformConstant
 %u8_ptr_uniform_constant = OpTypePointer UniformConstant %u8
 %u8_ptr_generic = OpTypePointer Generic %u8
+%u8_ptr_input = OpTypePointer Input %u8
 
 %main = OpFunction %void None %func
 %main_entry = OpLabel
@@ -5269,6 +5270,26 @@ TEST_F(ValidateExtInst, OpenCLStdPrintfFormatNotUniformConstStorageClass) {
                         "be UniformConstant"));
 }
 
+TEST_F(ValidateExtInst,
+       OpenCLStdPrintfFormatWithExtensionNotAllowedStorageClass) {
+  const std::string body = R"(
+%format_const = OpAccessChain %u8_ptr_uniform_constant %u8arr_uniform_constant %u32_0
+%format = OpBitcast %u8_ptr_input %format_const
+%val1 = OpExtInst %u32 %extinst printf %format %u32_0 %u32_1
+)";
+
+  const std::string extension = R"(
+OpExtension  "SPV_EXT_relaxed_printf_string_address_space"
+)";
+
+  CompileSuccessfully(GenerateKernelCode(body, extension));
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpenCL.std printf: expected Format storage class to "
+                        "be UniformConstant, Crossworkgroup, Workgroup, "
+                        "Function, or Generic"));
+}
+
 TEST_F(ValidateExtInst, OpenCLStdPrintfFormatNotU8Pointer) {
   const std::string body = R"(
 %format = OpAccessChain %u32_ptr_uniform_constant %u32vec8_uniform_constant %u32_0

utils/generate_grammar_tables.py

@@ -31,6 +31,7 @@
 SPV_AMD_gpu_shader_int16
 SPV_AMD_shader_trinary_minmax
 SPV_KHR_non_semantic_info
+SPV_EXT_relaxed_printf_string_address_space
 """
 
 OUTPUT_LANGUAGE = 'c'