From 3a7d68e4568ce98f7837fd162413fa7f2d24016f Mon Sep 17 00:00:00 2001 From: Robin Voetter Date: Sun, 23 Feb 2025 11:59:52 +0100 Subject: [PATCH 1/2] spirv-val: allow Float16 under OpenCL environments According to the OpenCL environment capability, the environments which allow the cl_khr_fp16 extension must allow the Float16 capability. Currently, however, the validator only allowed the Float16Buffer capability to be declared by the module. This commit adds the Float16 capability to the list of allowed optional capabilities. --- source/val/validate_capability.cpp | 1 + test/val/val_data_test.cpp | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/source/val/validate_capability.cpp b/source/val/validate_capability.cpp index 81d2ad52d2..99c0cb6814 100644 --- a/source/val/validate_capability.cpp +++ b/source/val/validate_capability.cpp @@ -221,6 +221,7 @@ bool IsSupportOptionalOpenCL_1_2(uint32_t capability) { switch (spv::Capability(capability)) { case spv::Capability::ImageBasic: case spv::Capability::Float64: + case spv::Capability::Float16: return true; default: break; diff --git a/test/val/val_data_test.cpp b/test/val/val_data_test.cpp index 349e5e9ef2..673d0dfe9d 100644 --- a/test/val/val_data_test.cpp +++ b/test/val/val_data_test.cpp @@ -89,6 +89,13 @@ std::string header_with_float64 = R"( OpCapability Float64 OpMemoryModel Logical GLSL450 )"; +std::string header_with_kernel_float16 = R"( + OpCapability Addresses + OpCapability Kernel + OpCapability Linkage + OpCapability Float16 + OpMemoryModel Physical32 OpenCL +)"; std::string invalid_comp_error = "Illegal number of components"; std::string missing_cap_error = "requires the Vector16 capability"; @@ -340,6 +347,12 @@ TEST_F(ValidateData, float16_buffer_good) { ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); } +TEST_F(ValidateData, float16_kernel_good) { + std::string str = header_with_kernel_float16 + "%2 = OpTypeFloat 16"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_OPENCL_1_2)); +} + TEST_F(ValidateData, float16_bad) { std::string str = header + "%2 = OpTypeFloat 16"; CompileSuccessfully(str.c_str()); From 5bd97b9613fe6eb2b74a68388414774139cc2ea5 Mon Sep 17 00:00:00 2001 From: Robin Voetter Date: Sun, 23 Feb 2025 13:05:39 +0100 Subject: [PATCH 2/2] spirv-val: Validate small types for Kernel Small type uses are the same for Shader modules as Kernel modules. Currently, the validation rules are disabled for any module which does not advertise the Shader capability. This commit removes this check, enabling the validator to also check these rules for OpenCL kernel modules. --- source/val/validate_small_type_uses.cpp | 2 +- test/val/val_small_type_uses_test.cpp | 79 +++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/source/val/validate_small_type_uses.cpp b/source/val/validate_small_type_uses.cpp index 69f61ee4f3..411a7b81c5 100644 --- a/source/val/validate_small_type_uses.cpp +++ b/source/val/validate_small_type_uses.cpp @@ -22,7 +22,7 @@ namespace val { spv_result_t ValidateSmallTypeUses(ValidationState_t& _, const Instruction* inst) { - if (!_.HasCapability(spv::Capability::Shader) || inst->type_id() == 0 || + if (inst->type_id() == 0 || !_.ContainsLimitedUseIntOrFloatType(inst->type_id())) { return SPV_SUCCESS; } diff --git a/test/val/val_small_type_uses_test.cpp b/test/val/val_small_type_uses_test.cpp index b950af5b01..84559934b2 100644 --- a/test/val/val_small_type_uses_test.cpp +++ b/test/val/val_small_type_uses_test.cpp @@ -333,6 +333,85 @@ INSTANTIATE_TEST_SUITE_P( "%inst = OpFunctionCall %void %half_func %ld_half", "%inst = OpFunctionCall %void %half_func %float_to_half")); +TEST_F(ValidateSmallTypeUses, F16OpsFail) { + const std::string body = R"( + OpCapability Addresses + OpCapability Kernel + OpCapability Float16Buffer + OpCapability Linkage + OpMemoryModel Physical64 OpenCL + %f16 = OpTypeFloat 16 +%func = OpTypeFunction %f16 %f16 %f16 + %add = OpFunction %f16 None %func + %a = OpFunctionParameter %f16 + %b = OpFunctionParameter %f16 + %lbl = OpLabel + %res = OpFAdd %f16 %a %b + OpReturnValue %res + OpFunctionEnd +)"; + + CompileSuccessfully(body.c_str()); + ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Invalid use of 8- or 16-bit result")); +} + +TEST_F(ValidateSmallTypeUses, F16OpsSuccess) { + const std::string body = R"( + OpCapability Addresses + OpCapability Kernel + OpCapability Float16Buffer + OpCapability Float16 + OpCapability Linkage + OpMemoryModel Physical64 OpenCL + %f16 = OpTypeFloat 16 +%func = OpTypeFunction %f16 %f16 %f16 + %add = OpFunction %f16 None %func + %a = OpFunctionParameter %f16 + %b = OpFunctionParameter %f16 + %lbl = OpLabel + %res = OpFAdd %f16 %a %b + OpReturnValue %res + OpFunctionEnd +)"; + + CompileSuccessfully(body.c_str()); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateSmallTypeUses, OpenCLVloadVstoreGood) { + // Using vload_half and vstore_half should be allowed + // with only Float16Buffer declared. + const std::string body = R"( + OpCapability Addresses + OpCapability Kernel + OpCapability Float16Buffer + OpCapability Linkage + OpCapability Int64 + %ocl = OpExtInstImport "OpenCL.std" + OpMemoryModel Physical64 OpenCL +%void = OpTypeVoid + %u64 = OpTypeInt 64 0 + %f16 = OpTypeFloat 16 + %f32 = OpTypeFloat 32 +%pf16 = OpTypePointer CrossWorkgroup %f16 +%func = OpTypeFunction %void %pf16 %pf16 + %idx = OpConstant %u64 1 + %add = OpFunction %void None %func + %src = OpFunctionParameter %pf16 + %dst = OpFunctionParameter %pf16 + %lbl = OpLabel + %a = OpExtInst %f32 %ocl vload_half %idx %src + %_ = OpExtInst %void %ocl vstore_half %a %idx %dst + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(body.c_str()); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + } // namespace } // namespace val } // namespace spvtools