From 0e80b86dbe1b6317bf3ea809df2cf0bc7f8064d5 Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Mon, 16 Apr 2018 10:05:11 -0400 Subject: [PATCH] Fixes #1472. Per-vertex variable validation fixes. Relaxs checks for per-vertex builtin variables. If the builtin decoration is applied to a variable, then those checks now allow a level of arraying on the variable before checking the type consistency. * Allows arrays of variables to be present for the per-vertex variables: * Position * PointSize * ClipDistance * CullDistance * Updated tests --- source/validate_builtins.cpp | 338 ++++++++++++++++++++++++++------- test/val/val_builtins_test.cpp | 138 +++++++++++++- 2 files changed, 406 insertions(+), 70 deletions(-) diff --git a/source/validate_builtins.cpp b/source/validate_builtins.cpp index bcd75dae1..b4d356d87 100644 --- a/source/validate_builtins.cpp +++ b/source/validate_builtins.cpp @@ -319,15 +319,40 @@ class BuiltInsValidator { spv_result_t ValidateF32( const Decoration& decoration, const Instruction& inst, const std::function& diag); + spv_result_t ValidateOptionalArrayedF32( + const Decoration& decoration, const Instruction& inst, + const std::function& diag); + spv_result_t ValidateF32Helper( + const Decoration& decoration, const Instruction& inst, + const std::function& diag, + uint32_t underlying_type); spv_result_t ValidateF32Vec( const Decoration& decoration, const Instruction& inst, uint32_t num_components, const std::function& diag); + spv_result_t ValidateOptionalArrayedF32Vec( + const Decoration& decoration, const Instruction& inst, + uint32_t num_components, + const std::function& diag); + spv_result_t ValidateF32VecHelper( + const Decoration& decoration, const Instruction& inst, + uint32_t num_components, + const std::function& diag, + uint32_t underlying_type); // If |num_components| is zero, the number of components is not checked. spv_result_t ValidateF32Arr( const Decoration& decoration, const Instruction& inst, uint32_t num_components, const std::function& diag); + spv_result_t ValidateOptionalArrayedF32Arr( + const Decoration& decoration, const Instruction& inst, + uint32_t num_components, + const std::function& diag); + spv_result_t ValidateF32ArrHelper( + const Decoration& decoration, const Instruction& inst, + uint32_t num_components, + const std::function& diag, + uint32_t underlying_type); // Generates strings like "Member #0 of struct ID <2>". std::string GetDefinitionDesc(const Decoration& decoration, @@ -520,6 +545,23 @@ spv_result_t BuiltInsValidator::ValidateI32( return SPV_SUCCESS; } +spv_result_t BuiltInsValidator::ValidateOptionalArrayedF32( + const Decoration& decoration, const Instruction& inst, + const std::function& diag) { + uint32_t underlying_type = 0; + if (spv_result_t error = + GetUnderlyingType(_, decoration, inst, &underlying_type)) { + return error; + } + + // Strip the array, if present. + if (_.GetIdOpcode(underlying_type) == SpvOpTypeArray) { + underlying_type = _.FindDef(underlying_type)->word(2u); + } + + return ValidateF32Helper(decoration, inst, diag, underlying_type); +} + spv_result_t BuiltInsValidator::ValidateF32( const Decoration& decoration, const Instruction& inst, const std::function& diag) { @@ -529,6 +571,13 @@ spv_result_t BuiltInsValidator::ValidateF32( return error; } + return ValidateF32Helper(decoration, inst, diag, underlying_type); +} + +spv_result_t BuiltInsValidator::ValidateF32Helper( + const Decoration& decoration, const Instruction& inst, + const std::function& diag, + uint32_t underlying_type) { if (!_.IsFloatScalarType(underlying_type)) { return diag(GetDefinitionDesc(decoration, inst) + " is not a float scalar."); @@ -578,6 +627,25 @@ spv_result_t BuiltInsValidator::ValidateI32Vec( return SPV_SUCCESS; } +spv_result_t BuiltInsValidator::ValidateOptionalArrayedF32Vec( + const Decoration& decoration, const Instruction& inst, + uint32_t num_components, + const std::function& diag) { + uint32_t underlying_type = 0; + if (spv_result_t error = + GetUnderlyingType(_, decoration, inst, &underlying_type)) { + return error; + } + + // Strip the array, if present. + if (_.GetIdOpcode(underlying_type) == SpvOpTypeArray) { + underlying_type = _.FindDef(underlying_type)->word(2u); + } + + return ValidateF32VecHelper(decoration, inst, num_components, diag, + underlying_type); +} + spv_result_t BuiltInsValidator::ValidateF32Vec( const Decoration& decoration, const Instruction& inst, uint32_t num_components, @@ -588,6 +656,15 @@ spv_result_t BuiltInsValidator::ValidateF32Vec( return error; } + return ValidateF32VecHelper(decoration, inst, num_components, diag, + underlying_type); +} + +spv_result_t BuiltInsValidator::ValidateF32VecHelper( + const Decoration& decoration, const Instruction& inst, + uint32_t num_components, + const std::function& diag, + uint32_t underlying_type) { if (!_.IsFloatVectorType(underlying_type)) { return diag(GetDefinitionDesc(decoration, inst) + " is not a float vector."); @@ -653,6 +730,37 @@ spv_result_t BuiltInsValidator::ValidateF32Arr( return error; } + return ValidateF32ArrHelper(decoration, inst, num_components, diag, + underlying_type); +} + +spv_result_t BuiltInsValidator::ValidateOptionalArrayedF32Arr( + const Decoration& decoration, const Instruction& inst, + uint32_t num_components, + const std::function& diag) { + uint32_t underlying_type = 0; + if (spv_result_t error = + GetUnderlyingType(_, decoration, inst, &underlying_type)) { + return error; + } + + // Strip an extra layer of arraying if present. + if (_.GetIdOpcode(underlying_type) == SpvOpTypeArray) { + uint32_t subtype = _.FindDef(underlying_type)->word(2u); + if (_.GetIdOpcode(subtype) == SpvOpTypeArray) { + underlying_type = subtype; + } + } + + return ValidateF32ArrHelper(decoration, inst, num_components, diag, + underlying_type); +} + +spv_result_t BuiltInsValidator::ValidateF32ArrHelper( + const Decoration& decoration, const Instruction& inst, + uint32_t num_components, + const std::function& diag, + uint32_t underlying_type) { const Instruction* const type_inst = _.FindDef(underlying_type); if (type_inst->opcode() != SpvOpTypeArray) { return diag(GetDefinitionDesc(decoration, inst) + " is not an array."); @@ -720,21 +828,6 @@ spv_result_t BuiltInsValidator::ValidateNotCalledWithExecutionModel( spv_result_t BuiltInsValidator::ValidateClipOrCullDistanceAtDefinition( const Decoration& decoration, const Instruction& inst) { - if (spvIsVulkanEnv(_.context()->target_env)) { - if (spv_result_t error = ValidateF32Arr( - decoration, inst, /* Any number of components */ 0, - [this, &decoration](const std::string& message) -> spv_result_t { - return _.diag(SPV_ERROR_INVALID_DATA) - << "According to the Vulkan spec BuiltIn " - << _.grammar().lookupOperandName(SPV_OPERAND_TYPE_BUILT_IN, - decoration.params()[0]) - << " variable needs to be a 32-bit float array. " - << message; - })) { - return error; - } - } - // Seed at reference checks with this built-in. return ValidateClipOrCullDistanceAtReference(decoration, inst, inst, inst); } @@ -784,11 +877,58 @@ spv_result_t BuiltInsValidator::ValidateClipOrCullDistanceAtReference( for (const SpvExecutionModel execution_model : execution_models_) { switch (execution_model) { case SpvExecutionModelFragment: - case SpvExecutionModelVertex: + case SpvExecutionModelVertex: { + if (spv_result_t error = ValidateF32Arr( + decoration, built_in_inst, /* Any number of components */ 0, + [this, + &decoration](const std::string& message) -> spv_result_t { + return _.diag(SPV_ERROR_INVALID_DATA) + << "According to the Vulkan spec BuiltIn " + << _.grammar().lookupOperandName( + SPV_OPERAND_TYPE_BUILT_IN, + decoration.params()[0]) + << " variable needs to be a 32-bit float array. " + << message; + })) { + return error; + } + break; + } case SpvExecutionModelTessellationControl: case SpvExecutionModelTessellationEvaluation: case SpvExecutionModelGeometry: { - // Ok. + if (decoration.struct_member_index() != Decoration::kInvalidMember) { + // The outer level of array is applied on the variable. + if (spv_result_t error = ValidateF32Arr( + decoration, built_in_inst, /* Any number of components */ 0, + [this, + &decoration](const std::string& message) -> spv_result_t { + return _.diag(SPV_ERROR_INVALID_DATA) + << "According to the Vulkan spec BuiltIn " + << _.grammar().lookupOperandName( + SPV_OPERAND_TYPE_BUILT_IN, + decoration.params()[0]) + << " variable needs to be a 32-bit float array. " + << message; + })) { + return error; + } + } else { + if (spv_result_t error = ValidateOptionalArrayedF32Arr( + decoration, built_in_inst, /* Any number of components */ 0, + [this, + &decoration](const std::string& message) -> spv_result_t { + return _.diag(SPV_ERROR_INVALID_DATA) + << "According to the Vulkan spec BuiltIn " + << _.grammar().lookupOperandName( + SPV_OPERAND_TYPE_BUILT_IN, + decoration.params()[0]) + << " variable needs to be a 32-bit float array. " + << message; + })) { + return error; + } + } break; } @@ -1287,19 +1427,6 @@ spv_result_t BuiltInsValidator::ValidatePointCoordAtReference( spv_result_t BuiltInsValidator::ValidatePointSizeAtDefinition( const Decoration& decoration, const Instruction& inst) { - if (spvIsVulkanEnv(_.context()->target_env)) { - if (spv_result_t error = ValidateF32( - decoration, inst, - [this](const std::string& message) -> spv_result_t { - return _.diag(SPV_ERROR_INVALID_DATA) - << "According to the Vulkan spec BuiltIn PointSize " - "variable needs to be a 32-bit float scalar. " - << message; - })) { - return error; - } - } - // Seed at reference checks with this built-in. return ValidatePointSizeAtReference(decoration, inst, inst, inst); } @@ -1333,11 +1460,51 @@ spv_result_t BuiltInsValidator::ValidatePointSizeAtReference( for (const SpvExecutionModel execution_model : execution_models_) { switch (execution_model) { - case SpvExecutionModelVertex: + case SpvExecutionModelVertex: { + if (spv_result_t error = ValidateF32( + decoration, built_in_inst, + [this](const std::string& message) -> spv_result_t { + return _.diag(SPV_ERROR_INVALID_DATA) + << "According to the Vulkan spec BuiltIn PointSize " + "variable needs to be a 32-bit float scalar. " + << message; + })) { + return error; + } + break; + } case SpvExecutionModelTessellationControl: case SpvExecutionModelTessellationEvaluation: case SpvExecutionModelGeometry: { - // Ok. + // PointSize can be a per-vertex variable for tessellation control, + // tessellation evaluation and geometry shader stages. In such cases + // variables will have an array of 32-bit floats. + if (decoration.struct_member_index() != Decoration::kInvalidMember) { + // The array is on the variable, so this must be a 32-bit float. + if (spv_result_t error = ValidateF32( + decoration, built_in_inst, + [this](const std::string& message) -> spv_result_t { + return _.diag(SPV_ERROR_INVALID_DATA) + << "According to the Vulkan spec BuiltIn " + "PointSize " + "variable needs to be a 32-bit float scalar. " + << message; + })) { + return error; + } + } else { + if (spv_result_t error = ValidateOptionalArrayedF32( + decoration, built_in_inst, + [this](const std::string& message) -> spv_result_t { + return _.diag(SPV_ERROR_INVALID_DATA) + << "According to the Vulkan spec BuiltIn " + "PointSize " + "variable needs to be a 32-bit float scalar. " + << message; + })) { + return error; + } + } break; } @@ -1365,20 +1532,6 @@ spv_result_t BuiltInsValidator::ValidatePointSizeAtReference( spv_result_t BuiltInsValidator::ValidatePositionAtDefinition( const Decoration& decoration, const Instruction& inst) { - if (spvIsVulkanEnv(_.context()->target_env)) { - if (spv_result_t error = ValidateF32Vec( - decoration, inst, 4, - [this](const std::string& message) -> spv_result_t { - return _.diag(SPV_ERROR_INVALID_DATA) - << "According to the Vulkan spec BuiltIn Position " - "variable needs to be a 4-component 32-bit float " - "vector. " - << message; - })) { - return error; - } - } - // Seed at reference checks with this built-in. return ValidatePositionAtReference(decoration, inst, inst, inst); } @@ -1412,19 +1565,61 @@ spv_result_t BuiltInsValidator::ValidatePositionAtReference( for (const SpvExecutionModel execution_model : execution_models_) { switch (execution_model) { - case SpvExecutionModelVertex: + case SpvExecutionModelVertex: { + if (spv_result_t error = ValidateF32Vec( + decoration, built_in_inst, 4, + [this](const std::string& message) -> spv_result_t { + return _.diag(SPV_ERROR_INVALID_DATA) + << "According to the Vulkan spec BuiltIn Position " + "variable needs to be a 4-component 32-bit float " + "vector. " + << message; + })) { + return error; + } + break; + } + case SpvExecutionModelGeometry: case SpvExecutionModelTessellationControl: - case SpvExecutionModelTessellationEvaluation: - case SpvExecutionModelGeometry: { - // Ok. + case SpvExecutionModelTessellationEvaluation: { + // Position can be a per-vertex variable for tessellation control, + // tessellation evaluation and geometry shader stages. In such cases + // variables will have an array of 4-component 32-bit float vectors. + if (decoration.struct_member_index() != Decoration::kInvalidMember) { + // The array is on the variable, so this must be a 4-component + // 32-bit float vector. + if (spv_result_t error = ValidateF32Vec( + decoration, built_in_inst, 4, + [this](const std::string& message) -> spv_result_t { + return _.diag(SPV_ERROR_INVALID_DATA) + << "According to the Vulkan spec BuiltIn Position " + "variable needs to be a 4-component 32-bit " + "float vector. " + << message; + })) { + return error; + } + } else { + if (spv_result_t error = ValidateOptionalArrayedF32Vec( + decoration, built_in_inst, 4, + [this](const std::string& message) -> spv_result_t { + return _.diag(SPV_ERROR_INVALID_DATA) + << "According to the Vulkan spec BuiltIn Position " + "variable needs to be a 4-component 32-bit " + "float vector. " + << message; + })) { + return error; + } + } break; } default: { return _.diag(SPV_ERROR_INVALID_DATA) - << "Vulkan spec allows BuiltIn Position to be used only with " - "Vertex, TessellationControl, TessellationEvaluation or " - "Geometry execution models. " + << "Vulkan spec allows BuiltIn Position to be used only " + "with Vertex, TessellationControl, TessellationEvaluation" + " or Geometry execution models. " << GetReferenceDesc(decoration, built_in_inst, referenced_inst, referenced_from_inst, execution_model); } @@ -1497,7 +1692,8 @@ spv_result_t BuiltInsValidator::ValidatePrimitiveIdAtReference( id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind( &BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, "Vulkan spec doesn't allow BuiltIn PrimitiveId to be used for " - "variables with Output storage class if execution model is Fragment.", + "variables with Output storage class if execution model is " + "Fragment.", SpvExecutionModelFragment, decoration, built_in_inst, referenced_from_inst, std::placeholders::_1)); } @@ -1629,7 +1825,8 @@ spv_result_t BuiltInsValidator::ValidateSampleMaskAtReference( for (const SpvExecutionModel execution_model : execution_models_) { if (execution_model != SpvExecutionModelFragment) { return _.diag(SPV_ERROR_INVALID_DATA) - << "Vulkan spec allows BuiltIn SampleMask to be used only with " + << "Vulkan spec allows BuiltIn SampleMask to be used only " + "with " "Fragment execution model. " << GetReferenceDesc(decoration, built_in_inst, referenced_inst, referenced_from_inst, execution_model); @@ -1676,7 +1873,8 @@ spv_result_t BuiltInsValidator::ValidateSamplePositionAtReference( if (storage_class != SpvStorageClassMax && storage_class != SpvStorageClassInput) { return _.diag(SPV_ERROR_INVALID_DATA) - << "Vulkan spec allows BuiltIn SamplePosition to be only used for " + << "Vulkan spec allows BuiltIn SamplePosition to be only used " + "for " "variables with Input storage class. " << GetReferenceDesc(decoration, built_in_inst, referenced_inst, referenced_from_inst) @@ -1826,7 +2024,8 @@ spv_result_t BuiltInsValidator::ValidateTessLevelAtReference( assert(function_id_ == 0); id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind( &BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, - "Vulkan spec doesn't allow TessLevelOuter/TessLevelInner to be used " + "Vulkan spec doesn't allow TessLevelOuter/TessLevelInner to be " + "used " "for variables with Input storage class if execution model is " "TessellationControl.", SpvExecutionModelTessellationControl, decoration, built_in_inst, @@ -1837,7 +2036,8 @@ spv_result_t BuiltInsValidator::ValidateTessLevelAtReference( assert(function_id_ == 0); id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind( &BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, - "Vulkan spec doesn't allow TessLevelOuter/TessLevelInner to be used " + "Vulkan spec doesn't allow TessLevelOuter/TessLevelInner to be " + "used " "for variables with Output storage class if execution model is " "TessellationEvaluation.", SpvExecutionModelTessellationEvaluation, decoration, built_in_inst, @@ -1914,7 +2114,8 @@ spv_result_t BuiltInsValidator::ValidateVertexIndexAtReference( for (const SpvExecutionModel execution_model : execution_models_) { if (execution_model != SpvExecutionModelVertex) { return _.diag(SPV_ERROR_INVALID_DATA) - << "Vulkan spec allows BuiltIn VertexIndex to be used only with " + << "Vulkan spec allows BuiltIn VertexIndex to be used only " + "with " "Vertex execution model. " << GetReferenceDesc(decoration, built_in_inst, referenced_inst, referenced_from_inst, execution_model); @@ -1976,8 +2177,10 @@ spv_result_t BuiltInsValidator::ValidateLayerOrViewportIndexAtReference( assert(function_id_ == 0); id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind( &BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, - "Vulkan spec doesn't allow BuiltIn Layer and ViewportIndex to be " - "used for variables with Input storage class if execution model is " + "Vulkan spec doesn't allow BuiltIn Layer and " + "ViewportIndex to be " + "used for variables with Input storage class if " + "execution model is " "Geometry.", SpvExecutionModelGeometry, decoration, built_in_inst, referenced_from_inst, std::placeholders::_1)); @@ -1987,8 +2190,10 @@ spv_result_t BuiltInsValidator::ValidateLayerOrViewportIndexAtReference( assert(function_id_ == 0); id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind( &BuiltInsValidator::ValidateNotCalledWithExecutionModel, this, - "Vulkan spec doesn't allow BuiltIn Layer and ViewportIndex to be " - "used for variables with Output storage class if execution model is " + "Vulkan spec doesn't allow BuiltIn Layer and " + "ViewportIndex to be " + "used for variables with Output storage class if " + "execution model is " "Fragment.", SpvExecutionModelFragment, decoration, built_in_inst, referenced_from_inst, std::placeholders::_1)); @@ -2097,7 +2302,8 @@ spv_result_t BuiltInsValidator::ValidateWorkgroupSizeAtDefinition( if (spvIsVulkanEnv(_.context()->target_env)) { if (!spvOpcodeIsConstant(inst.opcode())) { return _.diag(SPV_ERROR_INVALID_DATA) - << "Vulkan spec requires BuiltIn WorkgroupSize to be a constant. " + << "Vulkan spec requires BuiltIn WorkgroupSize to be a " + "constant. " << GetIdDesc(inst) << " is not a constant."; } @@ -2334,8 +2540,8 @@ spv_result_t BuiltInsValidator::Run() { continue; } - // Instruction references the id. Run all checks associated with the id on - // the instruction. id_to_at_reference_checks_ can be modified in the + // Instruction references the id. Run all checks associated with the id + // on the instruction. id_to_at_reference_checks_ can be modified in the // process, iterators are safe because it's a tree-based map. const auto it = id_to_at_reference_checks_.find(id); if (it != id_to_at_reference_checks_.end()) { diff --git a/test/val/val_builtins_test.cpp b/test/val/val_builtins_test.cpp index 0abc6be7b..c2e1041fe 100644 --- a/test/val/val_builtins_test.cpp +++ b/test/val/val_builtins_test.cpp @@ -49,6 +49,8 @@ using ValidateBuiltIns = spvtest::ValidateBase; using ValidateVulkanCombineBuiltInExecutionModelDataTypeResult = spvtest::ValidateBase>; +using ValidateVulkanCombineBuiltInArrayedVariable = spvtest::ValidateBase< + std::tuple>; struct EntryPoint { std::string name; @@ -186,6 +188,10 @@ std::string GetDefaultShaderTypes() { %f64arr2 = OpTypeArray %f64 %u32_2 %f64arr3 = OpTypeArray %f64 %u32_3 %f64arr4 = OpTypeArray %f64 %u32_4 + +%f32vec3arr3 = OpTypeArray %f32vec3 %u32_3 +%f32vec4arr3 = OpTypeArray %f32vec4 %u32_3 +%f64vec4arr3 = OpTypeArray %f64vec4 %u32_3 )"; } @@ -1474,6 +1480,128 @@ INSTANTIATE_TEST_CASE_P( "needs to be a 32-bit int scalar", "has bit width 64"))), ); +TEST_P(ValidateVulkanCombineBuiltInArrayedVariable, Variable) { + const char* const built_in = std::get<0>(GetParam()); + const char* const execution_model = std::get<1>(GetParam()); + const char* const storage_class = std::get<2>(GetParam()); + const char* const data_type = std::get<3>(GetParam()); + const TestResult& test_result = std::get<4>(GetParam()); + + CodeGenerator generator = GetDefaultShaderCodeGenerator(); + generator.before_types_ = "OpDecorate %built_in_var BuiltIn "; + generator.before_types_ += built_in; + generator.before_types_ += "\n"; + + std::ostringstream after_types; + after_types << "%built_in_array = OpTypeArray " << data_type << " %u32_3\n"; + after_types << "%built_in_ptr = OpTypePointer " << storage_class + << " %built_in_array\n"; + after_types << "%built_in_var = OpVariable %built_in_ptr " << storage_class + << "\n"; + generator.after_types_ = after_types.str(); + + EntryPoint entry_point; + entry_point.name = "main"; + entry_point.execution_model = execution_model; + // Any kind of reference would do. + entry_point.body = R"( +%val = OpBitcast %u64 %built_in_var +)"; + + std::ostringstream execution_modes; + if (0 == std::strcmp(execution_model, "Fragment")) { + execution_modes << "OpExecutionMode %" << entry_point.name + << " OriginUpperLeft\n"; + } + if (0 == std::strcmp(built_in, "FragDepth")) { + execution_modes << "OpExecutionMode %" << entry_point.name + << " DepthReplacing\n"; + } + entry_point.execution_modes = execution_modes.str(); + + generator.entry_points_.push_back(std::move(entry_point)); + + CompileSuccessfully(generator.Build(), SPV_ENV_VULKAN_1_0); + ASSERT_EQ(test_result.validation_result, + ValidateInstructions(SPV_ENV_VULKAN_1_0)); + if (test_result.error_str) { + EXPECT_THAT(getDiagnosticString(), HasSubstr(test_result.error_str)); + } + if (test_result.error_str2) { + EXPECT_THAT(getDiagnosticString(), HasSubstr(test_result.error_str2)); + } +} + +INSTANTIATE_TEST_CASE_P(PointSizeArrayedF32TessControl, + ValidateVulkanCombineBuiltInArrayedVariable, + Combine(Values("PointSize"), + Values("TessellationControl"), Values("Input"), + Values("%f32"), Values(TestResult())), ); + +INSTANTIATE_TEST_CASE_P( + PointSizeArrayedF64TessControl, ValidateVulkanCombineBuiltInArrayedVariable, + Combine(Values("PointSize"), Values("TessellationControl"), Values("Input"), + Values("%f64"), + Values(TestResult(SPV_ERROR_INVALID_DATA, + "needs to be a 32-bit float scalar", + "has bit width 64"))), ); + +INSTANTIATE_TEST_CASE_P( + PointSizeArrayedF32Vertex, ValidateVulkanCombineBuiltInArrayedVariable, + Combine(Values("PointSize"), Values("Vertex"), Values("Output"), + Values("%f32"), + Values(TestResult(SPV_ERROR_INVALID_DATA, + "needs to be a 32-bit float scalar", + "is not a float scalar"))), ); + +INSTANTIATE_TEST_CASE_P(PositionArrayedF32Vec4TessControl, + ValidateVulkanCombineBuiltInArrayedVariable, + Combine(Values("Position"), + Values("TessellationControl"), Values("Input"), + Values("%f32vec4"), Values(TestResult())), ); + +INSTANTIATE_TEST_CASE_P( + PositionArrayedF32Vec3TessControl, + ValidateVulkanCombineBuiltInArrayedVariable, + Combine(Values("Position"), Values("TessellationControl"), Values("Input"), + Values("%f32vec3"), + Values(TestResult(SPV_ERROR_INVALID_DATA, + "needs to be a 4-component 32-bit float vector", + "has 3 components"))), ); + +INSTANTIATE_TEST_CASE_P( + PositionArrayedF32Vec4Vertex, ValidateVulkanCombineBuiltInArrayedVariable, + Combine(Values("Position"), Values("Vertex"), Values("Output"), + Values("%f32"), + Values(TestResult(SPV_ERROR_INVALID_DATA, + "needs to be a 4-component 32-bit float vector", + "is not a float vector"))), ); + +INSTANTIATE_TEST_CASE_P( + ClipAndCullDistanceOutputSuccess, + ValidateVulkanCombineBuiltInArrayedVariable, + Combine(Values("ClipDistance", "CullDistance"), + Values("Geometry", "TessellationControl", "TessellationEvaluation"), + Values("Output"), Values("%f32arr2", "%f32arr4"), + Values(TestResult())), ); + +INSTANTIATE_TEST_CASE_P( + ClipAndCullDistanceVertexInput, ValidateVulkanCombineBuiltInArrayedVariable, + Combine(Values("ClipDistance", "CullDistance"), Values("Fragment"), + Values("Input"), Values("%f32arr4"), + Values(TestResult(SPV_ERROR_INVALID_DATA, + "needs to be a 32-bit float array", + "components are not float scalar"))), ); + +INSTANTIATE_TEST_CASE_P( + ClipAndCullDistanceNotArray, ValidateVulkanCombineBuiltInArrayedVariable, + Combine(Values("ClipDistance", "CullDistance"), + Values("Geometry", "TessellationControl", "TessellationEvaluation"), + Values("Input"), Values("%f32vec2", "%f32vec4"), + Values(TestResult(SPV_ERROR_INVALID_DATA, + "needs to be a 32-bit float array", + "components are not float scalar"))), ); + TEST_F(ValidateBuiltIns, WorkgroupSizeSuccess) { CodeGenerator generator = GetDefaultShaderCodeGenerator(); generator.before_types_ = R"( @@ -1689,11 +1817,13 @@ OpMemberDecorate %output_type 0 BuiltIn Position generator.after_types_ = R"( %input_type = OpTypeStruct %f32vec4 -%input_ptr = OpTypePointer Input %input_type +%arrayed_input_type = OpTypeArray %input_type %u32_3 +%input_ptr = OpTypePointer Input %arrayed_input_type %input = OpVariable %input_ptr Input %input_f32vec4_ptr = OpTypePointer Input %f32vec4 %output_type = OpTypeStruct %f32vec4 -%output_ptr = OpTypePointer Output %output_type +%arrayed_output_type = OpTypeArray %output_type %u32_3 +%output_ptr = OpTypePointer Output %arrayed_output_type %output = OpVariable %output_ptr Output %output_f32vec4_ptr = OpTypePointer Output %f32vec4 )"; @@ -1702,8 +1832,8 @@ OpMemberDecorate %output_type 0 BuiltIn Position entry_point.name = "main"; entry_point.execution_model = "Geometry"; entry_point.body = R"( -%input_pos = OpAccessChain %input_f32vec4_ptr %input %u32_0 -%output_pos = OpAccessChain %output_f32vec4_ptr %output %u32_0 +%input_pos = OpAccessChain %input_f32vec4_ptr %input %u32_0 %u32_0 +%output_pos = OpAccessChain %output_f32vec4_ptr %output %u32_0 %u32_0 %pos = OpLoad %f32vec4 %input_pos OpStore %output_pos %pos )";