spirv-reader: convert signedness of texturing result when needed
Fixed: tint:382
Change-Id: I786e1f5d5d122a82ef29c733e514bf3b45651c5d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35180
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index 607dc39..6a0c855 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -3940,9 +3940,32 @@
if (inst.type_id() != 0) {
// It returns a value.
+ ast::Expression* value = call_expr;
+
+ // If necessary, convert the result to the signedness of the instruction
+ // result type. Compare the SPIR-V image's sampled component type with the
+ // component of the result type of the SPIR-V instruction.
auto* result_type = parser_impl_.ConvertType(inst.type_id());
- // TODO(dneto): Convert result signedness if needed. crbug.com/tint/382
- EmitConstDefOrWriteToHoistedVar(inst, {result_type, call_expr});
+ auto* result_component_type = result_type;
+ if (auto* result_vector_type = result_type->As<ast::type::Vector>()) {
+ result_component_type = result_vector_type->type();
+ }
+ auto* spirv_image_type =
+ parser_impl_.GetSpirvTypeForHandleMemoryObjectDeclaration(*image);
+ if (!spirv_image_type || (spirv_image_type->opcode() != SpvOpTypeImage)) {
+ return Fail() << "invalid image type for image memory object declaration "
+ << image->PrettyPrint();
+ }
+ auto* expected_component_type =
+ parser_impl_.ConvertType(spirv_image_type->GetSingleWordInOperand(0));
+ if (expected_component_type != result_component_type) {
+ // This occurs if one is signed integer and the other is unsigned integer,
+ // or vice versa. Perform a bitcast.
+ value =
+ ast_module_.create<ast::BitcastExpression>(result_type, call_expr);
+ }
+
+ EmitConstDefOrWriteToHoistedVar(inst, {result_type, value});
} else {
// It's an image write. No value is returned, so make a statement out
// of the call.
diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc
index 3543a77..71e9319 100644
--- a/src/reader/spirv/parser_impl.cc
+++ b/src/reader/spirv/parser_impl.cc
@@ -1683,7 +1683,8 @@
}
}
-const spvtools::opt::Instruction* ParserImpl::GetSpirvTypeForHandleVar(
+const spvtools::opt::Instruction*
+ParserImpl::GetSpirvTypeForHandleMemoryObjectDeclaration(
const spvtools::opt::Instruction& var) {
if (!success()) {
return nullptr;
@@ -1700,14 +1701,16 @@
// Get the SPIR-V handle type.
const auto* ptr_type = def_use_mgr_->GetDef(var.type_id());
- if (!ptr_type) {
- Fail() << "Invalid type for variable " << var.PrettyPrint();
+ if (!ptr_type || (ptr_type->opcode() != SpvOpTypePointer)) {
+ Fail() << "Invalid type for variable or function parameter "
+ << var.PrettyPrint();
return nullptr;
}
const auto* raw_handle_type =
def_use_mgr_->GetDef(ptr_type->GetSingleWordInOperand(1));
if (!raw_handle_type) {
- Fail() << "Invalid pointer type for variable " << var.PrettyPrint();
+ Fail() << "Invalid pointer type for variable or function parameter "
+ << var.PrettyPrint();
return nullptr;
}
switch (raw_handle_type->opcode()) {
@@ -1719,11 +1722,12 @@
case SpvOpTypeRuntimeArray:
Fail()
<< "arrays of textures or samplers are not supported in WGSL; can't "
- "translate variable "
+ "translate variable or function parameter: "
<< var.PrettyPrint();
return nullptr;
default:
- Fail() << "invalid type for image or sampler variable: "
+ Fail() << "invalid type for image or sampler variable or function "
+ "parameter: "
<< var.PrettyPrint();
return nullptr;
}
@@ -1738,7 +1742,7 @@
}
const spvtools::opt::Instruction* raw_handle_type =
- GetSpirvTypeForHandleVar(var);
+ GetSpirvTypeForHandleMemoryObjectDeclaration(var);
if (!raw_handle_type) {
return nullptr;
}
diff --git a/src/reader/spirv/parser_impl.h b/src/reader/spirv/parser_impl.h
index 4d3b22a..2e9e9d7 100644
--- a/src/reader/spirv/parser_impl.h
+++ b/src/reader/spirv/parser_impl.h
@@ -431,11 +431,13 @@
Usage GetHandleUsage(uint32_t id) const;
/// Returns the SPIR-V type for the sampler or image type for the given
- /// variable in UniformConstant storage class. Returns null and emits an
+ /// variable in UniformConstant storage class, or function parameter pointing
+ /// into the UniformConstant storage class . Returns null and emits an
/// error on failure.
- /// @param var the OpVariable instruction
+ /// @param var the OpVariable instruction or OpFunctionParameter
/// @returns the Tint AST type for the sampler or texture, or null on error
- const spvtools::opt::Instruction* GetSpirvTypeForHandleVar(
+ const spvtools::opt::Instruction*
+ GetSpirvTypeForHandleMemoryObjectDeclaration(
const spvtools::opt::Instruction& var);
/// Returns the AST type for the pointer-to-sampler or pointer-to-texture type
diff --git a/src/reader/spirv/parser_impl_handle_test.cc b/src/reader/spirv/parser_impl_handle_test.cc
index cb40386..81726fe 100644
--- a/src/reader/spirv/parser_impl_handle_test.cc
+++ b/src/reader/spirv/parser_impl_handle_test.cc
@@ -1229,6 +1229,14 @@
OpName %vf12 "vf12"
OpName %vf123 "vf123"
OpName %vf1234 "vf1234"
+ OpName %u1 "u1"
+ OpName %vu12 "vu12"
+ OpName %vu123 "vu123"
+ OpName %vu1234 "vu1234"
+ OpName %i1 "i1"
+ OpName %vi12 "vi12"
+ OpName %vi123 "vi123"
+ OpName %vi1234 "vi1234"
OpName %coords1 "coords1"
OpName %coords12 "coords12"
OpName %coords123 "coords123"
@@ -1261,6 +1269,16 @@
%vf123 = OpCopyObject %v3float %the_vf123
%vf1234 = OpCopyObject %v4float %the_vf1234
+ %i1 = OpCopyObject %int %int_1
+ %vi12 = OpCopyObject %v2int %the_vi12
+ %vi123 = OpCopyObject %v3int %the_vi123
+ %vi1234 = OpCopyObject %v4int %the_vi1234
+
+ %u1 = OpCopyObject %uint %uint_1
+ %vu12 = OpCopyObject %v2uint %the_vu12
+ %vu123 = OpCopyObject %v3uint %the_vu123
+ %vu1234 = OpCopyObject %v4uint %the_vu1234
+
%coords1 = OpCopyObject %float %float_1
%coords12 = OpCopyObject %v2float %vf12
%coords123 = OpCopyObject %v3float %vf123
@@ -2889,7 +2907,7 @@
SpvParserTest_ImageAccessTest,
::testing::ValuesIn(std::vector<ImageAccessCase>{
// OpImageFetch with no extra params
- {"%float 2D 0 0 0 1 Rgba32f", "%99 = OpImageFetch %v4float %im %vu12",
+ {"%float 2D 0 0 0 1 Unknown", "%99 = OpImageFetch %v4float %im %vu12",
R"(DecoratedVariable{
Decorations{
SetDecoration{2}
@@ -2916,7 +2934,7 @@
}
})"},
// OpImageFetch with ConstOffset
- {"%float 2D 0 0 0 1 Rgba32f",
+ {"%float 2D 0 0 0 1 Unknown",
"%99 = OpImageFetch %v4float %im %vu12 ConstOffset %offsets2d",
R"(DecoratedVariable{
Decorations{
@@ -2946,6 +2964,502 @@
})"}}));
INSTANTIATE_TEST_SUITE_P(
+ ConvertResultSignedness,
+ SpvParserTest_SampledImageAccessTest,
+ ::testing::ValuesIn(std::vector<ImageAccessCase>{
+ // Valid SPIR-V only has:
+ // float scalar sampled type vs. floating result
+ // integral scalar sampled type vs. integral result
+ // Any of the sampling, reading, or fetching use the same codepath.
+
+ // We'll test with:
+ // OpImageFetch
+ // OpImageRead
+ // OpImageSampleImplicitLod - representative of sampling
+
+ //
+ // OpImageRead
+ //
+
+ // OpImageFetch requires no conversion, float -> v4float
+ {"%float 2D 0 0 0 1 Unknown", "%99 = OpImageFetch %v4float %im %vu12",
+ R"(DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __sampled_texture_2d__f32
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__f32
+ {
+ Call[not set]{
+ Identifier[not set]{textureLoad}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ })"},
+ // OpImageFetch requires no conversion, uint -> v4uint
+ {"%uint 2D 0 0 0 1 Unknown", "%99 = OpImageFetch %v4uint %im %vu12",
+ R"(DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __sampled_texture_2d__u32
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__u32
+ {
+ Call[not set]{
+ Identifier[not set]{textureLoad}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ })"},
+ // OpImageFetch requires conversion, uint -> v4int
+ {"%uint 2D 0 0 0 1 Unknown", "%99 = OpImageFetch %v4int %im %vu12",
+ R"(DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __sampled_texture_2d__u32
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__i32
+ {
+ Bitcast[not set]<__vec_4__i32>{
+ Call[not set]{
+ Identifier[not set]{textureLoad}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ }
+ })"},
+ // OpImageFetch requires no conversion, int -> v4int
+ {"%int 2D 0 0 0 1 Unknown", "%99 = OpImageFetch %v4int %im %vu12",
+ R"(DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __sampled_texture_2d__i32
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__i32
+ {
+ Call[not set]{
+ Identifier[not set]{textureLoad}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ })"},
+ // OpImageFetch requires conversion, int -> v4uint
+ {"%int 2D 0 0 0 1 Unknown", "%99 = OpImageFetch %v4uint %im %vu12",
+ R"(DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __sampled_texture_2d__i32
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__u32
+ {
+ Bitcast[not set]<__vec_4__u32>{
+ Call[not set]{
+ Identifier[not set]{textureLoad}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ }
+ })"},
+
+ //
+ // OpImageRead
+ //
+
+ // OpImageRead requires no conversion, float -> v4float
+ {"%float 2D 0 0 0 1 Rgba32f", "%99 = OpImageRead %v4float %im %vu12",
+ R"(DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __storage_texture_read_only_2d_rgba32float
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__f32
+ {
+ Call[not set]{
+ Identifier[not set]{textureLoad}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ })"},
+ // OpImageRead requires no conversion, uint -> v4uint
+ {"%uint 2D 0 0 0 1 Rgba32ui", "%99 = OpImageRead %v4uint %im %vu12",
+ R"(DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __storage_texture_read_only_2d_rgba32uint
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__u32
+ {
+ Call[not set]{
+ Identifier[not set]{textureLoad}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ })"},
+ // OpImageRead requires conversion, uint -> v4int
+ {"%uint 2D 0 0 0 1 Rgba32ui", "%99 = OpImageRead %v4int %im %vu12",
+ R"(DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __storage_texture_read_only_2d_rgba32uint
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__i32
+ {
+ Bitcast[not set]<__vec_4__i32>{
+ Call[not set]{
+ Identifier[not set]{textureLoad}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ }
+ })"},
+ // OpImageRead requires no conversion, int -> v4int
+ {"%int 2D 0 0 0 1 Rgba32i", "%99 = OpImageRead %v4int %im %vu12",
+ R"(DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __storage_texture_read_only_2d_rgba32sint
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__i32
+ {
+ Call[not set]{
+ Identifier[not set]{textureLoad}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ })"},
+ // OpImageRead requires conversion, int -> v4uint
+ {"%int 2D 0 0 0 1 Rgba32i", "%99 = OpImageRead %v4uint %im %vu12",
+ R"(DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __storage_texture_read_only_2d_rgba32sint
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__u32
+ {
+ Bitcast[not set]<__vec_4__u32>{
+ Call[not set]{
+ Identifier[not set]{textureLoad}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ }
+ })"},
+
+ //
+ // Sampling operations, using OpImageSampleImplicitLod as an example.
+ //
+
+ // OpImageSampleImplicitLod requires no conversion, float -> v4float
+ {"%float 2D 0 0 0 1 Unknown",
+ "%99 = OpImageSampleImplicitLod %v4float %sampled_image %vu12",
+ R"(
+ DecoratedVariable{
+ Decorations{
+ SetDecoration{0}
+ BindingDecoration{0}
+ }
+ x_10
+ uniform_constant
+ __sampler_sampler
+ }
+ DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __sampled_texture_2d__f32
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__f32
+ {
+ Call[not set]{
+ Identifier[not set]{textureSample}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{x_10}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ })"},
+ // OpImageSampleImplicitLod requires no conversion, uint -> v4uint
+ {"%uint 2D 0 0 0 1 Unknown",
+ "%99 = OpImageSampleImplicitLod %v4uint %sampled_image %vu12",
+ R"(
+ DecoratedVariable{
+ Decorations{
+ SetDecoration{0}
+ BindingDecoration{0}
+ }
+ x_10
+ uniform_constant
+ __sampler_sampler
+ }
+ DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __sampled_texture_2d__u32
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__u32
+ {
+ Call[not set]{
+ Identifier[not set]{textureSample}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{x_10}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ })"},
+ // OpImageSampleImplicitLod requires conversion, uint -> v4int
+ {"%uint 2D 0 0 0 1 Unknown",
+ "%99 = OpImageSampleImplicitLod %v4int %sampled_image %vu12",
+ R"(
+ DecoratedVariable{
+ Decorations{
+ SetDecoration{0}
+ BindingDecoration{0}
+ }
+ x_10
+ uniform_constant
+ __sampler_sampler
+ }
+ DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __sampled_texture_2d__u32
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__i32
+ {
+ Bitcast[not set]<__vec_4__i32>{
+ Call[not set]{
+ Identifier[not set]{textureSample}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{x_10}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ }
+ })"},
+ // OpImageSampleImplicitLod requires no conversion, int -> v4int
+ {"%int 2D 0 0 0 1 Unknown",
+ "%99 = OpImageSampleImplicitLod %v4int %sampled_image %vu12",
+ R"(
+ DecoratedVariable{
+ Decorations{
+ SetDecoration{0}
+ BindingDecoration{0}
+ }
+ x_10
+ uniform_constant
+ __sampler_sampler
+ }
+ DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __sampled_texture_2d__i32
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__i32
+ {
+ Call[not set]{
+ Identifier[not set]{textureSample}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{x_10}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ })"},
+ // OpImageSampleImplicitLod requires conversion, int -> v4uint
+ {"%int 2D 0 0 0 1 Unknown",
+ "%99 = OpImageSampleImplicitLod %v4uint %sampled_image %vu12",
+ R"(DecoratedVariable{
+ Decorations{
+ SetDecoration{2}
+ BindingDecoration{1}
+ }
+ x_20
+ uniform_constant
+ __sampled_texture_2d__i32
+ })",
+ R"(VariableDeclStatement{
+ VariableConst{
+ x_99
+ none
+ __vec_4__u32
+ {
+ Bitcast[not set]<__vec_4__u32>{
+ Call[not set]{
+ Identifier[not set]{textureSample}
+ (
+ Identifier[not set]{x_20}
+ Identifier[not set]{x_10}
+ Identifier[not set]{vu12}
+ )
+ }
+ }
+ }
+ }
+ })"}}));
+
+INSTANTIATE_TEST_SUITE_P(
// The SPIR-V result type could be integral but of different signedness
// than the sampled texel type. In these cases the result should be
// converted to match the signedness of the SPIR-V result type. This
@@ -3297,27 +3811,28 @@
}
)"}}}));
-INSTANTIATE_TEST_SUITE_P(BadInstructions,
- SpvParserTest_ImageCoordsTest,
- ::testing::ValuesIn(std::vector<ImageCoordsCase>{
- {"%float 1D 0 0 0 1 Unknown",
- "OpNop",
- "internal error: not an image access "
- "instruction: OpNop",
- {}},
- {"%float 1D 0 0 0 1 Unknown",
- "%50 = OpCopyObject %float %float_1",
- "internal error: couldn't find image for "
- "%50 = OpCopyObject %9 %36",
- {}},
- {"%float 1D 0 0 0 1 Unknown",
- "OpStore %float_var %float_1",
- "invalid type for image or sampler "
- "variable: %1 = OpVariable %2 Function",
- {}},
- // An example with a missing coordinate
- // won't assemble, so we skip it.
- }));
+INSTANTIATE_TEST_SUITE_P(
+ BadInstructions,
+ SpvParserTest_ImageCoordsTest,
+ ::testing::ValuesIn(std::vector<ImageCoordsCase>{
+ {"%float 1D 0 0 0 1 Unknown",
+ "OpNop",
+ "internal error: not an image access "
+ "instruction: OpNop",
+ {}},
+ {"%float 1D 0 0 0 1 Unknown",
+ "%50 = OpCopyObject %float %float_1",
+ "internal error: couldn't find image for "
+ "%50 = OpCopyObject %9 %36",
+ {}},
+ {"%float 1D 0 0 0 1 Unknown",
+ "OpStore %float_var %float_1",
+ "invalid type for image or sampler "
+ "variable or function parameter: %1 = OpVariable %2 Function",
+ {}},
+ // An example with a missing coordinate
+ // won't assemble, so we skip it.
+ }));
INSTANTIATE_TEST_SUITE_P(
Bad_Coordinate,
