From 41c8cd3e68aa9d42fdc8734d37fd738fe7b0dd0f Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Thu, 26 Nov 2020 13:57:32 +0000 Subject: [PATCH] writer/hlsl: Move the coord/arrayidx packing to utility function So we can also use this for the `spirv` backend Bug: tint:146 Change-Id: I26f70125a5015946d2428a6e669da32bdea23bcd Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/33780 Reviewed-by: David Neto Commit-Queue: Ben Clayton Auto-Submit: Ben Clayton --- BUILD.gn | 2 + src/CMakeLists.txt | 2 + src/writer/hlsl/generator_impl.cc | 54 +++----------------- src/writer/pack_coord_arrayidx.cc | 85 +++++++++++++++++++++++++++++++ src/writer/pack_coord_arrayidx.h | 52 +++++++++++++++++++ 5 files changed, 147 insertions(+), 48 deletions(-) create mode 100644 src/writer/pack_coord_arrayidx.cc create mode 100644 src/writer/pack_coord_arrayidx.h diff --git a/BUILD.gn b/BUILD.gn index 5199a59077..bf1d3932c8 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -426,6 +426,8 @@ source_set("libtint_core_src") { "src/validator/validator_test_helper.h", "src/writer/float_to_string.cc", "src/writer/float_to_string.h", + "src/writer/pack_coord_arrayidx.cc", + "src/writer/pack_coord_arrayidx.h", "src/writer/text.cc", "src/writer/text.h", "src/writer/text_generator.cc", diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 0ff1ee21ae..e41d3b7aed 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -247,6 +247,8 @@ set(TINT_LIB_SRCS validator/validator_test_helper.h writer/float_to_string.cc writer/float_to_string.h + writer/pack_coord_arrayidx.cc + writer/pack_coord_arrayidx.h writer/text.cc writer/text.h writer/text_generator.cc diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index 5eb1a3b349..86edcf7c2c 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc @@ -52,6 +52,7 @@ #include "src/ast/unary_op_expression.h" #include "src/ast/variable_decl_statement.h" #include "src/writer/float_to_string.h" +#include "src/writer/pack_coord_arrayidx.h" namespace tint { namespace writer { @@ -103,20 +104,6 @@ uint32_t convert_swizzle_to_index(const std::string& swizzle) { return 0; } -ast::TypeConstructorExpression* AsVectorConstructor(ast::Expression* expr) { - if (!expr->IsConstructor()) - return nullptr; - auto* constructor = expr->AsConstructor(); - if (!constructor->IsTypeConstructor()) { - return nullptr; - } - auto* type_constructor = constructor->AsTypeConstructor(); - if (!type_constructor->type()->IsVector()) { - return nullptr; - } - return type_constructor; -} - } // namespace GeneratorImpl::GeneratorImpl(Context* ctx, ast::Module* module) @@ -775,41 +762,12 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& pre, // Array index needs to be appended to the coordinates. auto* param_coords = params[pidx.coords]; auto* param_array_index = params[pidx.array_index]; - - uint32_t packed_coords_size; - ast::type::Type* packed_coords_el_ty; // Currenly must be f32. - if (param_coords->result_type()->IsVector()) { - auto* vec = param_coords->result_type()->AsVector(); - packed_coords_size = vec->size() + 1; - packed_coords_el_ty = vec->type(); - } else { - packed_coords_size = 2; - packed_coords_el_ty = param_coords->result_type(); - } - - // Cast param_array_index to the vector element type - ast::TypeConstructorExpression array_index_cast(packed_coords_el_ty, - {param_array_index}); - array_index_cast.set_result_type(packed_coords_el_ty); - - ast::type::VectorType packed_coords_ty(packed_coords_el_ty, - packed_coords_size); - - ast::ExpressionList coords; - // If the coordinates are already passed in a vector constructor, extract - // the elements into the new vector instead of nesting a vector-in-vector. - if (auto* vc = AsVectorConstructor(param_coords)) { - coords = vc->values(); - } else { - coords.emplace_back(param_coords); - } - coords.emplace_back(&array_index_cast); - - ast::TypeConstructorExpression constructor{&packed_coords_ty, - std::move(coords)}; - - if (!EmitExpression(pre, out, &constructor)) + if (!PackCoordAndArrayIndex(param_coords, param_array_index, + [&](ast::TypeConstructorExpression* packed) { + return EmitExpression(pre, out, packed); + })) { return false; + } } else { if (!EmitExpression(pre, out, params[pidx.coords])) diff --git a/src/writer/pack_coord_arrayidx.cc b/src/writer/pack_coord_arrayidx.cc new file mode 100644 index 0000000000..3384ee65c8 --- /dev/null +++ b/src/writer/pack_coord_arrayidx.cc @@ -0,0 +1,85 @@ +// Copyright 2020 The Tint Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "src/writer/pack_coord_arrayidx.h" + +#include + +#include "src/ast/expression.h" +#include "src/ast/type/vector_type.h" +#include "src/ast/type_constructor_expression.h" + +namespace tint { +namespace writer { + +namespace { + +ast::TypeConstructorExpression* AsVectorConstructor(ast::Expression* expr) { + if (!expr->IsConstructor()) + return nullptr; + auto* constructor = expr->AsConstructor(); + if (!constructor->IsTypeConstructor()) { + return nullptr; + } + auto* type_constructor = constructor->AsTypeConstructor(); + if (!type_constructor->type()->IsVector()) { + return nullptr; + } + return type_constructor; +} + +} // namespace + +bool PackCoordAndArrayIndex( + ast::Expression* coords, + ast::Expression* array_idx, + std::function callback) { + uint32_t packed_size; + ast::type::Type* packed_el_ty; // Currenly must be f32. + if (coords->result_type()->IsVector()) { + auto* vec = coords->result_type()->AsVector(); + packed_size = vec->size() + 1; + packed_el_ty = vec->type(); + } else { + packed_size = 2; + packed_el_ty = coords->result_type(); + } + + if (!packed_el_ty) { + return false; // missing type info + } + + // Cast array_idx to the vector element type + ast::TypeConstructorExpression array_index_cast(packed_el_ty, {array_idx}); + array_index_cast.set_result_type(packed_el_ty); + + ast::type::VectorType packed_ty(packed_el_ty, packed_size); + + // If the coordinates are already passed in a vector constructor, extract + // the elements into the new vector instead of nesting a vector-in-vector. + ast::ExpressionList packed; + if (auto* vc = AsVectorConstructor(coords)) { + packed = vc->values(); + } else { + packed.emplace_back(coords); + } + packed.emplace_back(&array_index_cast); + + ast::TypeConstructorExpression constructor{&packed_ty, std::move(packed)}; + + return callback(&constructor); +} + +} // namespace writer +} // namespace tint diff --git a/src/writer/pack_coord_arrayidx.h b/src/writer/pack_coord_arrayidx.h new file mode 100644 index 0000000000..d31728a059 --- /dev/null +++ b/src/writer/pack_coord_arrayidx.h @@ -0,0 +1,52 @@ +// Copyright 2020 The Tint Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef SRC_WRITER_PACK_COORD_ARRAYIDX_H_ +#define SRC_WRITER_PACK_COORD_ARRAYIDX_H_ + +#include + +#include "src/source.h" + +namespace tint { + +namespace ast { +class Expression; +class TypeConstructorExpression; +} // namespace ast + +namespace writer { + +/// A helper function use to generate texture intrinsic function calls for +/// backends that expect the texture coordinate and array index to be packed +/// together into a single 'coordinate' parameter. +/// PackCoordAndArrayIndex() calls the @p callback function with a vector +/// expression containing the elements of @p coords followed by the single +/// element of @p array_idx cast to the @p coords element type. +/// All types must have been assigned to the expressions and their child nodes +/// before calling. +/// @param coords the texture coordinates. May be a scalar, `vec2` or `vec3`. +/// @param array_idx the texture array index. Must be a scalar. +/// @param callback the function called with the packed result. Note that the +/// pointer argument is only valid for the duration of the call. +/// @returns the value returned by `callback` to indicate success +bool PackCoordAndArrayIndex( + ast::Expression* coords, + ast::Expression* array_idx, + std::function callback); + +} // namespace writer +} // namespace tint + +#endif // SRC_WRITER_PACK_COORD_ARRAYIDX_H_