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.

Sign up for GitHub

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

Jump to bottom

spirv-val: Add Vulkan Image Format check #5458

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

spirv-val: Add Vulkan Image Format check #5458

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
952da96
spirv-val: Add Vulkan Image Format check
spencer-lunarg Oct 30, 2023
5b66a41
spirv-val: Add Check for valid SignExtend/ZeroExtend
spencer-lunarg Oct 31, 2023
7895cbf
spirv-val: Fix naming for ImageFormat functions
spencer-lunarg Nov 1, 2023
03698f1
spirv-val: Collapse ZeroExtend error message
spencer-lunarg Nov 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
161 changes: 157 additions & 4 deletions source/val/validate_image.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,97 @@ bool IsValidGatherLodBiasAMD(const ValidationState_t& _, spv::Op opcode) {
return false;
}

bool IsImageFormatInt32Width(spv::ImageFormat format) {
spencer-lunarg marked this conversation as resolved.
Show resolved Hide resolved
switch (format) {
case spv::ImageFormat::Rgba32i:
case spv::ImageFormat::Rgba16i:
case spv::ImageFormat::Rgba8i:
case spv::ImageFormat::R32i:
case spv::ImageFormat::Rg32i:
case spv::ImageFormat::Rg16i:
case spv::ImageFormat::Rg8i:
case spv::ImageFormat::R16i:
case spv::ImageFormat::R8i:
case spv::ImageFormat::Rgba32ui:
case spv::ImageFormat::Rgba16ui:
case spv::ImageFormat::Rgba8ui:
case spv::ImageFormat::R32ui:
case spv::ImageFormat::Rgb10a2ui:
case spv::ImageFormat::Rg32ui:
case spv::ImageFormat::Rg16ui:
case spv::ImageFormat::Rg8ui:
case spv::ImageFormat::R16ui:
case spv::ImageFormat::R8ui:
return true;
default:
break;
}
return false;
}

bool IsImageFormatInt64Width(spv::ImageFormat format) {
spencer-lunarg marked this conversation as resolved.
Show resolved Hide resolved
switch (format) {
case spv::ImageFormat::R64ui:
case spv::ImageFormat::R64i:
return true;
default:
break;
}
return false;
}

bool IsImageFormatIntSigned(spv::ImageFormat format) {
switch (format) {
case spv::ImageFormat::Rgba32i:
case spv::ImageFormat::Rgba16i:
case spv::ImageFormat::Rgba8i:
case spv::ImageFormat::R32i:
case spv::ImageFormat::Rg32i:
case spv::ImageFormat::Rg16i:
case spv::ImageFormat::Rg8i:
case spv::ImageFormat::R16i:
case spv::ImageFormat::R8i:
case spv::ImageFormat::R64i:
return true;
default:
break;
}
return false;
}

bool IsImageFormatFloat(spv::ImageFormat format) {
switch (format) {
case spv::ImageFormat::Rgba32f:
case spv::ImageFormat::Rgba16f:
case spv::ImageFormat::R32f:
case spv::ImageFormat::Rgba8:
case spv::ImageFormat::Rgba8Snorm:
case spv::ImageFormat::Rg32f:
case spv::ImageFormat::Rg16f:
case spv::ImageFormat::R11fG11fB10f:
case spv::ImageFormat::R16f:
case spv::ImageFormat::Rgba16:
case spv::ImageFormat::Rgb10A2:
case spv::ImageFormat::Rg16:
case spv::ImageFormat::Rg8:
case spv::ImageFormat::R16:
case spv::ImageFormat::R8:
case spv::ImageFormat::Rgba16Snorm:
case spv::ImageFormat::Rg16Snorm:
case spv::ImageFormat::Rg8Snorm:
case spv::ImageFormat::R16Snorm:
case spv::ImageFormat::R8Snorm:
return true;
default:
break;
}
return false;
}

bool IsImageFormatInt(spv::ImageFormat format) {
return IsImageFormatInt32Width(format) || IsImageFormatInt64Width(format);
}

// Returns true if the opcode is a Image instruction which applies
// homogenous projection to the coordinates.
bool IsProj(spv::Op opcode) {
Expand Down Expand Up @@ -255,6 +346,8 @@ spv_result_t ValidateImageOperands(ValidationState_t& _,
const bool have_explicit_mask = (word_index - 1 < num_words);
const uint32_t mask = have_explicit_mask ? inst->word(word_index - 1) : 0u;

const auto target_env = _.context()->target_env;

if (have_explicit_mask) {
// NonPrivate, Volatile, SignExtend, ZeroExtend take no operand words.
const uint32_t mask_bits_having_operands =
Expand Down Expand Up @@ -287,6 +380,41 @@ spv_result_t ValidateImageOperands(ValidationState_t& _,
"multi-sampled image";
}

// The following OpTypeImage checks are done here as they depend of if the
// SignExtend and ZeroExtend are used to override the signedness
const bool is_sign_extend =
mask & uint32_t(spv::ImageOperandsMask::SignExtend);
const bool is_zero_extend =
mask & uint32_t(spv::ImageOperandsMask::ZeroExtend);
if (spvIsVulkanEnv(target_env)) {
if (info.format != spv::ImageFormat::Unknown &&
_.IsIntScalarType(info.sampled_type)) {
const bool is_format_signed = IsImageFormatIntSigned(info.format);
// (vkspec.html#spirvenv-image-signedness) has order signedness is set by
bool is_sampled_type_signed =
is_sign_extend
? true
: (is_zero_extend
? false
: (_.IsSignedIntScalarType(info.sampled_type) ? true
: false));
if (is_format_signed != is_sampled_type_signed) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< _.VkErrorID(4965)
<< "Image Format signedness does not match Sample Type operand "
"including possible SignExtend or ZeroExtend operand";
}
}

// Format might be Unknown, so check type
if (_.IsFloatScalarType(info.sampled_type) &&
(is_sign_extend || is_zero_extend)) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< _.VkErrorID(4965)
<< "Can't use SignExtend or ZeroExtend on float images";
}
}

// After this point, only set bits in the image operands mask can cause
// the module to be invalid.
if (mask == 0) return SPV_SUCCESS;
Expand Down Expand Up @@ -454,8 +582,7 @@ spv_result_t ValidateImageOperands(ValidationState_t& _,
<< " components, but given " << offset_size;
}

if (!_.options()->before_hlsl_legalization &&
spvIsVulkanEnv(_.context()->target_env)) {
if (!_.options()->before_hlsl_legalization && spvIsVulkanEnv(target_env)) {
if (opcode != spv::Op::OpImageGather &&
opcode != spv::Op::OpImageDrefGather &&
opcode != spv::Op::OpImageSparseGather &&
Expand Down Expand Up @@ -610,7 +737,7 @@ spv_result_t ValidateImageOperands(ValidationState_t& _,
if (auto error = ValidateMemoryScope(_, inst, visible_scope)) return error;
}

if (mask & uint32_t(spv::ImageOperandsMask::SignExtend)) {
if (is_sign_extend) {
// Checked elsewhere: SPIR-V 1.4 version or later.

// "The texel value is converted to the target value via sign extension.
Expand All @@ -623,7 +750,7 @@ spv_result_t ValidateImageOperands(ValidationState_t& _,
// setup.
}

if (mask & uint32_t(spv::ImageOperandsMask::ZeroExtend)) {
if (is_zero_extend) {
// Checked elsewhere: SPIR-V 1.4 version or later.

// "The texel value is converted to the target value via zero extension.
Expand Down Expand Up @@ -916,6 +1043,32 @@ spv_result_t ValidateTypeImage(ValidationState_t& _, const Instruction* inst) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< _.VkErrorID(6214) << "Dim SubpassData requires Arrayed to be 0";
}

// Can't check signedness here due to image operands able to override
// sampled type
if (info.format != spv::ImageFormat::Unknown) {
// validated above so can assume this is a 32-bit float, 32-bit int, or
// 64-bit int
const bool is_int = _.IsIntScalarType(info.sampled_type);
const bool is_float = !is_int;
if ((is_float && !IsImageFormatFloat(info.format)) ||
(is_int && !IsImageFormatInt(info.format))) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< _.VkErrorID(4965)
<< "Image Format type (float or int) does not match Sample Type "
"operand";
}
if (is_int) {
const uint32_t bit_width = _.GetBitWidth(info.sampled_type);
if ((bit_width == 32 && !IsImageFormatInt32Width(info.format)) ||
(bit_width == 64 && !IsImageFormatInt64Width(info.format))) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< _.VkErrorID(4965)
<< "Image Format width (32 or 64) does not match Sample Type "
"operand";
}
}
}
}

return SPV_SUCCESS;
Expand Down
2 changes: 2 additions & 0 deletions source/val/validation_state.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2225,6 +2225,8 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
return VUID_WRAP(VUID-StandaloneSpirv-Component-04923);
case 4924:
return VUID_WRAP(VUID-StandaloneSpirv-Component-04924);
case 4965:
return VUID_WRAP(VUID-StandaloneSpirv-Image-04965);
case 6201:
return VUID_WRAP(VUID-StandaloneSpirv-Flat-06201);
case 6202:
Expand Down