spirv-reader: Apply image coord vector check on unwrapped type

When validating the image coordinate type of an identifier that may have
been hoisted into a 'var' declaration, spirv-reader correctly checked
the type of the unwrapped reference for scalars but not when the
coordinate type is a vector.

This change applies the vector type checks to the unwrapped type.
Introduced a vector coordinate variant of the
SpvParserHandleTest.ImageCoordinateCanBeHoistedConstant test which
demonstrates the issue and passes with the fix.

Fixed: tint:1712
Change-Id: I9d99a1996e5df71921d6f66d1af02fb5088f1f20
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/116371
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: David Neto <dneto@google.com>
This commit is contained in:
Arman Uguray 2023-01-09 20:00:51 +00:00 committed by Dawn LUCI CQ
parent 07c32cb938
commit 2dfd177009
2 changed files with 88 additions and 1 deletions

View File

@ -5979,7 +5979,7 @@ FunctionEmitter::ExpressionList FunctionEmitter::MakeCoordinateOperandsForImageA
auto* component_type = raw_coords.type->UnwrapRef();
if (component_type->IsFloatScalar() || component_type->IsIntegerScalar()) {
num_coords_supplied = 1;
} else if (auto* vec_type = As<Vector>(raw_coords.type)) {
} else if (auto* vec_type = As<Vector>(component_type)) {
component_type = vec_type->type;
num_coords_supplied = vec_type->size;
}

View File

@ -3914,6 +3914,93 @@ return;
ASSERT_EQ(expect, got);
}
TEST_F(SpvParserHandleTest, ImageCoordinateCanBeHoistedVector) {
// Demonstrates fix for crbug.com/tint/1712
// The problem is the coordinate for an image operation
// can be a combinatorial value that has been hoisted out
// to a 'var' declaration.
//
// In this test (and the original case form the bug), the
// definition for the value is in an outer construct, and
// the image operation using it is in a doubly nested
// construct.
//
// The coordinate handling has to unwrap the ref type it
// made for the 'var' declaration.
const auto assembly = Preamble() + R"(
OpEntryPoint Fragment %100 "main"
OpExecutionMode %100 OriginUpperLeft
OpDecorate %10 DescriptorSet 0
OpDecorate %10 Binding 0
OpDecorate %20 DescriptorSet 2
OpDecorate %20 Binding 1
%void = OpTypeVoid
%voidfn = OpTypeFunction %void
%bool = OpTypeBool
%true = OpConstantTrue %bool
%float = OpTypeFloat 32
%v2float = OpTypeVector %float 2
%v4float = OpTypeVector %float 4
%v2float_null = OpConstantNull %v2float
%sampler = OpTypeSampler
%ptr_sampler = OpTypePointer UniformConstant %sampler
%im_ty = OpTypeImage %float 1D 0 0 0 1 Unknown
%ptr_im_ty = OpTypePointer UniformConstant %im_ty
%si_ty = OpTypeSampledImage %im_ty
%10 = OpVariable %ptr_sampler UniformConstant
%20 = OpVariable %ptr_im_ty UniformConstant
%100 = OpFunction %void None %voidfn
%entry = OpLabel
%900 = OpCopyObject %v2float %v2float_null ; definition here
OpSelectionMerge %99 None
OpBranchConditional %true %40 %99
%40 = OpLabel
OpSelectionMerge %80 None
OpBranchConditional %true %50 %80
%50 = OpLabel
%sam = OpLoad %sampler %10
%im = OpLoad %im_ty %20
%sampled_image = OpSampledImage %si_ty %im %sam
%result = OpImageSampleImplicitLod %v4float %sampled_image %900 ; usage here
OpBranch %80
%80 = OpLabel
OpBranch %99
%99 = OpLabel
OpReturn
OpFunctionEnd
)";
auto p = parser(test::Assemble(assembly));
EXPECT_TRUE(p->BuildAndParseInternalModule()) << assembly;
auto fe = p->function_emitter(100);
EXPECT_TRUE(fe.EmitBody()) << p->error();
EXPECT_TRUE(p->error().empty()) << p->error();
auto ast_body = fe.ast_body();
const auto got = test::ToString(p->program(), ast_body);
auto* expect = R"(var x_900 : vec2<f32>;
x_900 = vec2<f32>();
if (true) {
if (true) {
let x_19 : vec4<f32> = textureSample(x_20, x_10, x_900.x);
}
}
return;
)";
ASSERT_EQ(expect, got);
}
TEST_F(SpvParserHandleTest, TexelTypeWhenLoop) {
// Demonstrates fix for crbug.com/tint/1642
// The problem is the texel value for an image write