Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SPV_KHR_fragment_shader_barycentric support #4805

Merged
merged 2 commits into from
May 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions source/opt/aggressive_dead_code_elim_pass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -968,6 +968,7 @@ void AggressiveDCEPass::InitExtensions() {
"SPV_EXT_shader_image_int64",
"SPV_KHR_non_semantic_info",
"SPV_KHR_uniform_group_instructions",
"SPV_KHR_fragment_shader_barycentric",
});
}

Expand Down
1 change: 1 addition & 0 deletions source/opt/local_access_chain_convert_pass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ void LocalAccessChainConvertPass::InitExtensions() {
"SPV_EXT_shader_image_int64",
"SPV_KHR_non_semantic_info",
"SPV_KHR_uniform_group_instructions",
"SPV_KHR_fragment_shader_barycentric",
});
}

Expand Down
1 change: 1 addition & 0 deletions source/opt/local_single_block_elim_pass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ void LocalSingleBlockLoadStoreElimPass::InitExtensions() {
"SPV_EXT_shader_image_int64",
"SPV_KHR_non_semantic_info",
"SPV_KHR_uniform_group_instructions",
"SPV_KHR_fragment_shader_barycentric",
});
}

Expand Down
1 change: 1 addition & 0 deletions source/opt/local_single_store_elim_pass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ void LocalSingleStoreElimPass::InitExtensionAllowList() {
"SPV_EXT_shader_image_int64",
"SPV_KHR_non_semantic_info",
"SPV_KHR_uniform_group_instructions",
"SPV_KHR_fragment_shader_barycentric",
});
}
bool LocalSingleStoreElimPass::ProcessVariable(Instruction* var_inst) {
Expand Down
4 changes: 2 additions & 2 deletions source/val/validate_annotation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ std::string LogStringForDecoration(uint32_t decoration) {
return "PerViewNV";
case SpvDecorationPerTaskNV:
return "PerTaskNV";
case SpvDecorationPerVertexNV:
return "PerVertexNV";
case SpvDecorationPerVertexKHR:
return "PerVertexKHR";
case SpvDecorationNonUniform:
return "NonUniform";
case SpvDecorationRestrictPointer:
Expand Down
95 changes: 91 additions & 4 deletions source/val/validate_builtins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ typedef enum VUIDError_ {
VUIDErrorMax,
} VUIDError;

const static uint32_t NumVUIDBuiltins = 34;
const static uint32_t NumVUIDBuiltins = 36;

typedef struct {
SpvBuiltIn builtIn;
Expand Down Expand Up @@ -163,6 +163,8 @@ std::array<BuiltinVUIDMapping, NumVUIDBuiltins> builtinVUIDInfo = {{
{SpvBuiltInFragStencilRefEXT, {4223, 4224, 4225}},
{SpvBuiltInFullyCoveredEXT, {4232, 4233, 4234}},
{SpvBuiltInCullMaskKHR, {6735, 6736, 6737}},
{SpvBuiltInBaryCoordKHR, {4154, 4155, 4156}},
{SpvBuiltInBaryCoordNoPerspKHR, {4160, 4161, 4162}},
// clang-format off
} };

Expand Down Expand Up @@ -333,7 +335,9 @@ class BuiltInsValidator {
const Decoration& decoration, const Instruction& inst);
spv_result_t ValidateSMBuiltinsAtDefinition(const Decoration& decoration,
const Instruction& inst);

// Used for BaryCoord, BaryCoordNoPersp.
spv_result_t ValidateFragmentShaderF32Vec3InputAtDefinition(
const Decoration& decoration, const Instruction& inst);
// Used for SubgroupEqMask, SubgroupGeMask, SubgroupGtMask, SubgroupLtMask,
// SubgroupLeMask.
spv_result_t ValidateI32Vec4InputAtDefinition(const Decoration& decoration,
Expand Down Expand Up @@ -511,6 +515,13 @@ class BuiltInsValidator {
const Decoration& decoration, const Instruction& built_in_inst,
const Instruction& referenced_inst,
const Instruction& referenced_from_inst);

// Used for BaryCoord, BaryCoordNoPersp.
spv_result_t ValidateFragmentShaderF32Vec3InputAtReference(
const Decoration& decoration, const Instruction& built_in_inst,
const Instruction& referenced_inst,
const Instruction& referenced_from_inst);

// Used for SubgroupId and NumSubgroups.
spv_result_t ValidateComputeI32InputAtReference(
const Decoration& decoration, const Instruction& built_in_inst,
Expand Down Expand Up @@ -2790,6 +2801,80 @@ spv_result_t BuiltInsValidator::ValidateLayerOrViewportIndexAtReference(
return SPV_SUCCESS;
}

spv_result_t BuiltInsValidator::ValidateFragmentShaderF32Vec3InputAtDefinition(
const Decoration& decoration, const Instruction& inst) {
if (spvIsVulkanEnv(_.context()->target_env)) {
const SpvBuiltIn builtin = SpvBuiltIn(decoration.params()[0]);
if (spv_result_t error = ValidateF32Vec(
decoration, inst, 3,
[this, &inst, builtin](const std::string& message) -> spv_result_t {
uint32_t vuid = GetVUIDForBuiltin(builtin, VUIDErrorType);
return _.diag(SPV_ERROR_INVALID_DATA, &inst)
<< _.VkErrorID(vuid) << "According to the "
<< spvLogStringForEnv(_.context()->target_env)
<< " spec BuiltIn "
<< _.grammar().lookupOperandName(SPV_OPERAND_TYPE_BUILT_IN,
builtin)
<< " variable needs to be a 3-component 32-bit float "
"vector. "
<< message;
})) {
return error;
}
}

// Seed at reference checks with this built-in.
return ValidateFragmentShaderF32Vec3InputAtReference(decoration, inst, inst,
inst);
}

spv_result_t BuiltInsValidator::ValidateFragmentShaderF32Vec3InputAtReference(
const Decoration& decoration, const Instruction& built_in_inst,
const Instruction& referenced_inst,
const Instruction& referenced_from_inst) {

if (spvIsVulkanEnv(_.context()->target_env)) {
const SpvBuiltIn builtin = SpvBuiltIn(decoration.params()[0]);
const SpvStorageClass storage_class = GetStorageClass(referenced_from_inst);
if (storage_class != SpvStorageClassMax &&
storage_class != SpvStorageClassInput) {
uint32_t vuid = GetVUIDForBuiltin(builtin, VUIDErrorStorageClass);
return _.diag(SPV_ERROR_INVALID_DATA, &referenced_from_inst)
<< _.VkErrorID(vuid) << spvLogStringForEnv(_.context()->target_env)
<< " spec allows BuiltIn "
<< _.grammar().lookupOperandName(SPV_OPERAND_TYPE_BUILT_IN, builtin)
<< " to be only used for variables with Input storage class. "
<< GetReferenceDesc(decoration, built_in_inst, referenced_inst,
referenced_from_inst)
<< " " << GetStorageClassDesc(referenced_from_inst);
}

for (const SpvExecutionModel execution_model : execution_models_) {
if (execution_model != SpvExecutionModelFragment) {
uint32_t vuid = GetVUIDForBuiltin(builtin, VUIDErrorExecutionModel);
return _.diag(SPV_ERROR_INVALID_DATA, &referenced_from_inst)
<< _.VkErrorID(vuid)
<< spvLogStringForEnv(_.context()->target_env)
<< " spec allows BuiltIn "
<< _.grammar().lookupOperandName(SPV_OPERAND_TYPE_BUILT_IN, builtin)
<< " to be used only with Fragment execution model. "
<< GetReferenceDesc(decoration, built_in_inst, referenced_inst,
referenced_from_inst, execution_model);
}
}
}

if (function_id_ == 0) {
// Propagate this rule to all dependant ids in the global scope.
id_to_at_reference_checks_[referenced_from_inst.id()].push_back(std::bind(
&BuiltInsValidator::ValidateFragmentShaderF32Vec3InputAtReference, this,
decoration, built_in_inst, referenced_from_inst,
std::placeholders::_1));
}

return SPV_SUCCESS;
}

spv_result_t BuiltInsValidator::ValidateComputeShaderI32Vec3InputAtDefinition(
const Decoration& decoration, const Instruction& inst) {
if (spvIsVulkanEnv(_.context()->target_env)) {
Expand Down Expand Up @@ -4030,6 +4115,10 @@ spv_result_t BuiltInsValidator::ValidateSingleBuiltInAtDefinition(
case SpvBuiltInWorkgroupId: {
return ValidateComputeShaderI32Vec3InputAtDefinition(decoration, inst);
}
case SpvBuiltInBaryCoordKHR:
case SpvBuiltInBaryCoordNoPerspKHR: {
return ValidateFragmentShaderF32Vec3InputAtDefinition(decoration, inst);
}
case SpvBuiltInHelperInvocation: {
return ValidateHelperInvocationAtDefinition(decoration, inst);
}
Expand Down Expand Up @@ -4186,8 +4275,6 @@ spv_result_t BuiltInsValidator::ValidateSingleBuiltInAtDefinition(
case SpvBuiltInLayerPerViewNV:
case SpvBuiltInMeshViewCountNV:
case SpvBuiltInMeshViewIndicesNV:
case SpvBuiltInBaryCoordNV:
case SpvBuiltInBaryCoordNoPerspNV:
case SpvBuiltInCurrentRayTimeNV:
// No validation rules (for the moment).
break;
Expand Down
8 changes: 4 additions & 4 deletions source/val/validate_interfaces.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ spv_result_t GetLocationsForVariable(
uint32_t index = 0;
bool has_patch = false;
bool has_per_task_nv = false;
bool has_per_vertex_nv = false;
bool has_per_vertex_khr = false;
for (auto& dec : _.id_decorations(variable->id())) {
if (dec.dec_type() == SpvDecorationLocation) {
if (has_location && dec.params()[0] != location) {
Expand Down Expand Up @@ -272,8 +272,8 @@ spv_result_t GetLocationsForVariable(
has_patch = true;
} else if (dec.dec_type() == SpvDecorationPerTaskNV) {
has_per_task_nv = true;
} else if (dec.dec_type() == SpvDecorationPerVertexNV) {
has_per_vertex_nv = true;
} else if (dec.dec_type() == SpvDecorationPerVertexKHR) {
has_per_vertex_khr = true;
}
}

Expand All @@ -298,7 +298,7 @@ spv_result_t GetLocationsForVariable(
}
break;
case SpvExecutionModelFragment:
if (!is_output && has_per_vertex_nv) {
if (!is_output && has_per_vertex_khr) {
is_arrayed = true;
}
break;
Expand Down
12 changes: 12 additions & 0 deletions source/val/validation_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1411,6 +1411,18 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
// Clang format adds spaces between hyphens
// clang-format off
switch (id) {
case 4154:
return VUID_WRAP(VUID-BaryCoordKHR-BaryCoordKHR-04154);
case 4155:
return VUID_WRAP(VUID-BaryCoordKHR-BaryCoordKHR-04155);
case 4156:
return VUID_WRAP(VUID-BaryCoordKHR-BaryCoordKHR-04156);
case 4160:
return VUID_WRAP(VUID-BaryCoordNoPerspKHR-BaryCoordNoPerspKHR-04160);
case 4161:
return VUID_WRAP(VUID-BaryCoordNoPerspKHR-BaryCoordNoPerspKHR-04161);
case 4162:
return VUID_WRAP(VUID-BaryCoordNoPerspKHR-BaryCoordNoPerspKHR-04162);
case 4181:
return VUID_WRAP(VUID-BaseInstance-BaseInstance-04181);
case 4182:
Expand Down
65 changes: 65 additions & 0 deletions test/val/val_builtins_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4126,6 +4126,71 @@ INSTANTIATE_TEST_SUITE_P(
"According to the Vulkan spec BuiltIn FullyCoveredEXT variable "
"needs to be a bool scalar."))));

INSTANTIATE_TEST_SUITE_P(
BaryCoordNotFragment,
ValidateVulkanCombineBuiltInExecutionModelDataTypeCapabilityExtensionResult,
Combine(
Values("BaryCoordKHR", "BaryCoordNoPerspKHR"), Values("Vertex"),
Values("Input"), Values("%f32vec3"),
Values("OpCapability FragmentBarycentricKHR\n"),
Values("OpExtension \"SPV_KHR_fragment_shader_barycentric\"\n"),
Values("VUID-BaryCoordKHR-BaryCoordKHR-04154 "
"VUID-BaryCoordNoPerspKHR-BaryCoordNoPerspKHR-04160 "),
Values(TestResult(SPV_ERROR_INVALID_DATA, "Vulkan spec allows BuiltIn",
"to be used only with Fragment execution model"))));

INSTANTIATE_TEST_SUITE_P(
BaryCoordNotInput,
ValidateVulkanCombineBuiltInExecutionModelDataTypeCapabilityExtensionResult,
Combine(Values("BaryCoordKHR", "BaryCoordNoPerspKHR"), Values("Fragment"),
Values("Output"), Values("%f32vec3"),
Values("OpCapability FragmentBarycentricKHR\n"),
Values("OpExtension \"SPV_KHR_fragment_shader_barycentric\"\n"),
Values("VUID-BaryCoordKHR-BaryCoordKHR-04155 "
"VUID-BaryCoordNoPerspKHR-BaryCoordNoPerspKHR-04161 "),
Values(TestResult(
SPV_ERROR_INVALID_DATA, "Vulkan spec allows BuiltIn",
"to be only used for variables with Input storage class"))));

INSTANTIATE_TEST_SUITE_P(
BaryCoordNotFloatVector,
ValidateVulkanCombineBuiltInExecutionModelDataTypeCapabilityExtensionResult,
Combine(
Values("BaryCoordKHR", "BaryCoordNoPerspKHR"), Values("Fragment"),
Values("Output"), Values("%f32arr3", "%u32vec4"),
Values("OpCapability FragmentBarycentricKHR\n"),
Values("OpExtension \"SPV_KHR_fragment_shader_barycentric\"\n"),
Values("VUID-BaryCoordKHR-BaryCoordKHR-04156 "
"VUID-BaryCoordNoPerspKHR-BaryCoordNoPerspKHR-04162 "),
Values(TestResult(SPV_ERROR_INVALID_DATA,
"needs to be a 3-component 32-bit float vector"))));

INSTANTIATE_TEST_SUITE_P(
BaryCoordNotFloatVec3,
ValidateVulkanCombineBuiltInExecutionModelDataTypeCapabilityExtensionResult,
Combine(
Values("BaryCoordKHR", "BaryCoordNoPerspKHR"), Values("Fragment"),
Values("Output"), Values("%f32vec2"),
Values("OpCapability FragmentBarycentricKHR\n"),
Values("OpExtension \"SPV_KHR_fragment_shader_barycentric\"\n"),
Values("VUID-BaryCoordKHR-BaryCoordKHR-04156 "
"VUID-BaryCoordNoPerspKHR-BaryCoordNoPerspKHR-04162 "),
Values(TestResult(SPV_ERROR_INVALID_DATA,
"needs to be a 3-component 32-bit float vector"))));

INSTANTIATE_TEST_SUITE_P(
BaryCoordNotF32Vec3,
ValidateVulkanCombineBuiltInExecutionModelDataTypeCapabilityExtensionResult,
Combine(
Values("BaryCoordKHR", "BaryCoordNoPerspKHR"), Values("Fragment"),
Values("Output"), Values("%f64vec3"),
Values("OpCapability FragmentBarycentricKHR\n"),
Values("OpExtension \"SPV_KHR_fragment_shader_barycentric\"\n"),
Values("VUID-BaryCoordKHR-BaryCoordKHR-04156 "
"VUID-BaryCoordNoPerspKHR-BaryCoordNoPerspKHR-04162 "),
Values(TestResult(SPV_ERROR_INVALID_DATA,
"needs to be a 3-component 32-bit float vector"))));

} // namespace
} // namespace val
} // namespace spvtools