Skip to content

Commit 8ac4c4f

Browse files
spirv-val: Combine Image Coordinate checks (#6494)
This originally came out of https://gitlab.khronos.org/spirv/SPIR-V/-/issues/905 where we found a driver that crashed passing it 16-bit floats as the coordinate I tried to combine the logic and went through each type ... I am not sure why/if there is a difference between Vulkan/OpenCL with respect to allowing Float vs Int (but found 1 that we have active tests for https://gitlab.khronos.org/spirv/SPIR-V/-/issues/908) ... the other is `OpImageRead`/`OpImageWrite`, those were being validated incorrectly according to the spec
1 parent 58224f5 commit 8ac4c4f

File tree

5 files changed

+276
-205
lines changed

5 files changed

+276
-205
lines changed

source/opt/convert_to_half_pass.cpp

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ namespace spvtools {
2222
namespace opt {
2323
namespace {
2424
// Indices of operands in SPIR-V instructions
25+
constexpr int kImageSampleCoordinateIdInIdx = 1;
2526
constexpr int kImageSampleDrefIdInIdx = 2;
2627
} // namespace
2728

@@ -325,7 +326,7 @@ bool ConvertToHalfPass::ProcessConvert(Instruction* inst) {
325326

326327
bool ConvertToHalfPass::ProcessImageRef(Instruction* inst) {
327328
bool modified = false;
328-
// If image reference, only need to convert dref args back to float32
329+
// If image reference, some operands aren't allowed to be non-32 bit floats
329330
if (dref_image_ops_.count(inst->opcode()) != 0) {
330331
uint32_t dref_id = inst->GetSingleWordInOperand(kImageSampleDrefIdInIdx);
331332
if (converted_ids_.count(dref_id) > 0) {
@@ -338,6 +339,19 @@ bool ConvertToHalfPass::ProcessImageRef(Instruction* inst) {
338339
modified = true;
339340
}
340341
}
342+
if (coordinate_image_ops_.count(inst->opcode()) != 0) {
343+
uint32_t coordinate_id =
344+
inst->GetSingleWordInOperand(kImageSampleCoordinateIdInIdx);
345+
if (converted_ids_.count(coordinate_id) > 0) {
346+
GenConvert(&coordinate_id, 32, inst);
347+
if (status_ == Status::Failure) {
348+
return false;
349+
}
350+
inst->SetInOperand(kImageSampleCoordinateIdInIdx, {coordinate_id});
351+
get_def_use_mgr()->AnalyzeInstUse(inst);
352+
modified = true;
353+
}
354+
}
341355
return modified;
342356
}
343357

@@ -591,6 +605,30 @@ void ConvertToHalfPass::Initialize() {
591605
spv::Op::OpImageSparseSampleProjDrefExplicitLod,
592606
spv::Op::OpImageSparseDrefGather,
593607
};
608+
coordinate_image_ops_ = {
609+
spv::Op::OpImageSampleImplicitLod,
610+
spv::Op::OpImageSampleExplicitLod,
611+
spv::Op::OpImageSampleDrefImplicitLod,
612+
spv::Op::OpImageSampleDrefExplicitLod,
613+
spv::Op::OpImageSampleProjImplicitLod,
614+
spv::Op::OpImageSampleProjExplicitLod,
615+
spv::Op::OpImageSampleProjDrefImplicitLod,
616+
spv::Op::OpImageSampleProjDrefExplicitLod,
617+
spv::Op::OpImageFetch,
618+
spv::Op::OpImageGather,
619+
spv::Op::OpImageDrefGather,
620+
spv::Op::OpImageRead,
621+
spv::Op::OpImageWrite,
622+
spv::Op::OpImageQueryLod,
623+
spv::Op::OpImageSparseSampleImplicitLod,
624+
spv::Op::OpImageSparseSampleExplicitLod,
625+
spv::Op::OpImageSparseSampleDrefImplicitLod,
626+
spv::Op::OpImageSparseSampleDrefExplicitLod,
627+
spv::Op::OpImageSparseFetch,
628+
spv::Op::OpImageSparseGather,
629+
spv::Op::OpImageSparseDrefGather,
630+
spv::Op::OpImageSparseRead,
631+
};
594632
closure_ops_ = {
595633
spv::Op::OpVectorExtractDynamic,
596634
spv::Op::OpVectorInsertDynamic,

source/opt/convert_to_half_pass.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,9 @@ class ConvertToHalfPass : public Pass {
145145
// Set of only dref sample operations
146146
std::unordered_set<spv::Op, hasher> dref_image_ops_;
147147

148+
// Set of only sample operations that have a Coordinate operand
149+
std::unordered_set<spv::Op, hasher> coordinate_image_ops_;
150+
148151
// Set of operations that can be marked as relaxed
149152
std::unordered_set<spv::Op, hasher> closure_ops_;
150153

0 commit comments

Comments
 (0)