diff --git a/src/tint/utils/hash.h b/src/tint/utils/hash.h index e043e81fe8..5ca663b56f 100644 --- a/src/tint/utils/hash.h +++ b/src/tint/utils/hash.h @@ -18,6 +18,7 @@ #include #include #include +#include #include namespace tint::utils { @@ -76,6 +77,53 @@ size_t Hash(const ARGS&... args) { return hash; } +/// Wrapper for a hashable type enabling the wrapped value to be used as a key +/// for an unordered_map or unordered_set. +template +struct UnorderedKeyWrapper { + /// The wrapped value + const T value; + /// The hash of value + const size_t hash; + + /// Constructor + /// @param v the value to wrap + explicit UnorderedKeyWrapper(const T& v) : value(v), hash(Hash(v)) {} + + /// Move constructor + /// @param v the value to wrap + explicit UnorderedKeyWrapper(T&& v) + : value(std::move(v)), hash(Hash(value)) {} + + /// @returns true if this wrapper comes before other + /// @param other the RHS of the operator + bool operator<(const UnorderedKeyWrapper& other) const { + return hash < other.hash; + } + + /// @returns true if this wrapped value is equal to the other wrapped value + /// @param other the RHS of the operator + bool operator==(const UnorderedKeyWrapper& other) const { + return value == other.value; + } +}; + } // namespace tint::utils +namespace std { + +/// Custom std::hash specialization for tint::utils::UnorderedKeyWrapper +template +class hash> { + public: + /// @param w the UnorderedKeyWrapper + /// @return the hash value + inline std::size_t operator()( + const tint::utils::UnorderedKeyWrapper& w) const { + return w.hash; + } +}; + +} // namespace std + #endif // SRC_TINT_UTILS_HASH_H_ diff --git a/src/tint/utils/hash_test.cc b/src/tint/utils/hash_test.cc index ec88a8ca23..3f7289012a 100644 --- a/src/tint/utils/hash_test.cc +++ b/src/tint/utils/hash_test.cc @@ -15,6 +15,7 @@ #include "src/tint/utils/hash.h" #include +#include #include "gtest/gtest.h" @@ -43,5 +44,30 @@ TEST(HashTests, Vector) { Hash(std::vector({1, 2, 3, 4}))); } +TEST(HashTests, UnorderedKeyWrapper) { + using W = UnorderedKeyWrapper>; + + std::unordered_map m; + + m.emplace(W{{1, 2}}, -1); + EXPECT_EQ(m.size(), 1u); + EXPECT_EQ(m[W({1, 2})], -1); + + m.emplace(W{{3, 2}}, 1); + EXPECT_EQ(m.size(), 2u); + EXPECT_EQ(m[W({3, 2})], 1); + EXPECT_EQ(m[W({1, 2})], -1); + + m.emplace(W{{100}}, 100); + EXPECT_EQ(m.size(), 3u); + EXPECT_EQ(m[W({100})], 100); + EXPECT_EQ(m[W({3, 2})], 1); + EXPECT_EQ(m[W({1, 2})], -1); + + // Reversed vector element order + EXPECT_EQ(m[W({2, 3})], 0); + EXPECT_EQ(m[W({2, 1})], 0); +} + } // namespace } // namespace tint::utils diff --git a/src/tint/writer/spirv/builder.cc b/src/tint/writer/spirv/builder.cc index c3c677b9ce..cf98b5d64e 100644 --- a/src/tint/writer/spirv/builder.cc +++ b/src/tint/writer/spirv/builder.cc @@ -1346,9 +1346,6 @@ uint32_t Builder::GenerateTypeConstructorOrConversion( return GenerateConstantNullIfNeeded(result_type->UnwrapRef()); } - std::ostringstream out; - out << "__const_" << result_type->FriendlyName(builder_.Symbols()) << "_"; - result_type = result_type->UnwrapRef(); bool constructor_is_const = IsConstructorConst(call->Declaration()); if (has_error()) { @@ -1386,6 +1383,12 @@ uint32_t Builder::GenerateTypeConstructorOrConversion( } OperandList ops; + static constexpr size_t kOpsResultIdx = 1; + static constexpr size_t kOpsFirstValueIdx = 2; + ops.reserve(8); + ops.push_back(Operand::Int(type_id)); + ops.push_back(Operand::Int(0)); // Placeholder for the result ID + for (auto* e : args) { uint32_t id = 0; id = GenerateExpressionWithLoadIfNeeded(e); @@ -1399,8 +1402,6 @@ uint32_t Builder::GenerateTypeConstructorOrConversion( // value type is a correctly sized vector so we can just use it directly. if (result_type == value_type || result_type->Is() || result_type->Is() || result_type->Is()) { - out << "_" << id; - ops.push_back(Operand::Int(id)); continue; } @@ -1410,7 +1411,6 @@ uint32_t Builder::GenerateTypeConstructorOrConversion( if (value_type->is_scalar() && result_type->is_scalar()) { id = GenerateCastOrCopyOrPassthrough(result_type, args[0]->Declaration(), global_var); - out << "_" << id; ops.push_back(Operand::Int(id)); continue; } @@ -1461,7 +1461,6 @@ uint32_t Builder::GenerateTypeConstructorOrConversion( result_is_spec_composite = true; } - out << "_" << extract_id; ops.push_back(Operand::Int(extract_id)); } } else { @@ -1476,33 +1475,27 @@ uint32_t Builder::GenerateTypeConstructorOrConversion( args[0]->Type()->UnwrapRef()->is_scalar()) { size_t vec_size = init_result_type->As()->Width(); for (size_t i = 0; i < (vec_size - 1); ++i) { - ops.push_back(ops[0]); + ops.push_back(ops[kOpsFirstValueIdx]); } } - auto str = out.str(); - auto val = type_constructor_to_id_.find(str); - if (val != type_constructor_to_id_.end()) { - return val->second; - } + return utils::GetOrCreate( + type_constructor_to_id_, OperandListKey{ops}, [&]() -> uint32_t { + auto result = result_op(); + ops[kOpsResultIdx] = result; - auto result = result_op(); - ops.insert(ops.begin(), result); - ops.insert(ops.begin(), Operand::Int(type_id)); + if (result_is_spec_composite) { + push_type(spv::Op::OpSpecConstantComposite, ops); + } else if (result_is_constant_composite) { + push_type(spv::Op::OpConstantComposite, ops); + } else { + if (!push_function_inst(spv::Op::OpCompositeConstruct, ops)) { + return 0; + } + } - type_constructor_to_id_[str] = result.to_i(); - - if (result_is_spec_composite) { - push_type(spv::Op::OpSpecConstantComposite, ops); - } else if (result_is_constant_composite) { - push_type(spv::Op::OpConstantComposite, ops); - } else { - if (!push_function_inst(spv::Op::OpCompositeConstruct, ops)) { - return 0; - } - } - - return result.to_i(); + return result.to_i(); + }); } uint32_t Builder::GenerateCastOrCopyOrPassthrough( diff --git a/src/tint/writer/spirv/builder.h b/src/tint/writer/spirv/builder.h index 1605d47bbf..cf07e27156 100644 --- a/src/tint/writer/spirv/builder.h +++ b/src/tint/writer/spirv/builder.h @@ -604,7 +604,7 @@ class Builder { std::unordered_map func_sig_to_id_; std::unordered_map type_to_id_; std::unordered_map const_to_id_; - std::unordered_map type_constructor_to_id_; + std::unordered_map type_constructor_to_id_; std::unordered_map const_null_to_id_; std::unordered_map const_splat_to_id_; std::unordered_map diff --git a/src/tint/writer/spirv/operand.cc b/src/tint/writer/spirv/operand.cc index 7d2724997c..726c780b72 100644 --- a/src/tint/writer/spirv/operand.cc +++ b/src/tint/writer/spirv/operand.cc @@ -19,21 +19,21 @@ namespace tint::writer::spirv { // static Operand Operand::Float(float val) { Operand o(Kind::kFloat); - o.set_float(val); + o.float_val_ = val; return o; } // static Operand Operand::Int(uint32_t val) { Operand o(Kind::kInt); - o.set_int(val); + o.int_val_ = val; return o; } // static Operand Operand::String(const std::string& val) { Operand o(Kind::kString); - o.set_string(val); + o.str_val_ = val; return o; } diff --git a/src/tint/writer/spirv/operand.h b/src/tint/writer/spirv/operand.h index 46a5deb55b..fbc0c404c8 100644 --- a/src/tint/writer/spirv/operand.h +++ b/src/tint/writer/spirv/operand.h @@ -15,9 +15,12 @@ #ifndef SRC_TINT_WRITER_SPIRV_OPERAND_H_ #define SRC_TINT_WRITER_SPIRV_OPERAND_H_ +#include #include #include +#include "src/tint/utils/hash.h" + namespace tint::writer::spirv { /// A single SPIR-V instruction operand @@ -53,16 +56,29 @@ class Operand { /// @returns a copy of this operand Operand& operator=(const Operand& b) = default; - /// Sets the float value - /// @param val the value to set - void set_float(float val) { float_val_ = val; } - /// Sets the int value - /// @param val the value to set - void set_int(uint32_t val) { int_val_ = val; } - /// Sets the string value - /// @param val the value to set - void set_string(const std::string& val) { str_val_ = val; } + /// Equality operator + /// @param other the RHS of the operator + /// @returns true if this operand is equal to other + bool operator==(const Operand& other) const { + if (kind_ == other.kind_) { + switch (kind_) { + case tint::writer::spirv::Operand::Kind::kFloat: + // Use memcmp to work around: + // error: comparing floating point with == or != is unsafe + // [-Werror,-Wfloat-equal] + return memcmp(&float_val_, &other.float_val_, sizeof(float_val_)) == + 0; + case tint::writer::spirv::Operand::Kind::kInt: + return int_val_ == other.int_val_; + case tint::writer::spirv::Operand::Kind::kString: + return str_val_ == other.str_val_; + } + } + return false; + } + /// @returns the kind of the operand + Kind GetKind() const { return kind_; } /// @returns true if this is a float operand bool IsFloat() const { return kind_ == Kind::kFloat; } /// @returns true if this is an integer operand @@ -90,6 +106,30 @@ class Operand { /// A list of operands using OperandList = std::vector; +using OperandListKey = utils::UnorderedKeyWrapper; + } // namespace tint::writer::spirv +namespace std { + +/// Custom std::hash specialization for tint::writer::spirv::Operand +template <> +class hash { + public: + /// @param o the Operand + /// @return the hash value + inline std::size_t operator()(const tint::writer::spirv::Operand& o) const { + switch (o.GetKind()) { + case tint::writer::spirv::Operand::Kind::kFloat: + return tint::utils::Hash(o.to_f()); + case tint::writer::spirv::Operand::Kind::kInt: + return tint::utils::Hash(o.to_i()); + case tint::writer::spirv::Operand::Kind::kString: + return tint::utils::Hash(o.to_s()); + } + return 0; + } +}; + +} // namespace std #endif // SRC_TINT_WRITER_SPIRV_OPERAND_H_ diff --git a/test/tint/types/function_scope_var_conversions.wgsl.expected.spvasm b/test/tint/types/function_scope_var_conversions.wgsl.expected.spvasm index 1bf5ab9cb9..5eee4dfe13 100644 --- a/test/tint/types/function_scope_var_conversions.wgsl.expected.spvasm +++ b/test/tint/types/function_scope_var_conversions.wgsl.expected.spvasm @@ -1,7 +1,7 @@ ; SPIR-V ; Version: 1.3 ; Generator: Google Tint Compiler; 0 -; Bound: 73 +; Bound: 72 ; Schema: 0 OpCapability Shader OpMemoryModel Logical GLSL450 @@ -69,12 +69,11 @@ %_ptr_Function_v3uint = OpTypePointer Function %v3uint %61 = OpConstantNull %v3uint %63 = OpConstantComposite %v3uint %uint_1 %uint_1 %uint_1 - %65 = OpConstantComposite %v3bool %true %true %true %v4bool = OpTypeVector %bool 4 %false = OpConstantFalse %bool - %69 = OpConstantComposite %v4bool %true %false %true %false + %68 = OpConstantComposite %v4bool %true %false %true %false %_ptr_Function_v4bool = OpTypePointer Function %v4bool - %72 = OpConstantNull %v4bool + %71 = OpConstantNull %v4bool %constant_with_non_constant = OpFunction %void None %1 %4 = OpLabel %a = OpVariable %_ptr_Function_float Function %6 @@ -107,7 +106,7 @@ %v3u32_var2 = OpVariable %_ptr_Function_v3uint Function %61 %v3u32_var3 = OpVariable %_ptr_Function_v3uint Function %61 %v3bool_var4 = OpVariable %_ptr_Function_v3bool Function %45 -%v4bool_var5 = OpVariable %_ptr_Function_v4bool Function %72 +%v4bool_var5 = OpVariable %_ptr_Function_v4bool Function %71 OpStore %bool_var1 %true OpStore %bool_var2 %true OpStore %bool_var3 %true @@ -127,7 +126,7 @@ OpStore %v3u32_var1 %58 OpStore %v3u32_var2 %58 OpStore %v3u32_var3 %63 - OpStore %v3bool_var4 %65 - OpStore %v4bool_var5 %69 + OpStore %v3bool_var4 %42 + OpStore %v4bool_var5 %68 OpReturn OpFunctionEnd diff --git a/test/tint/types/module_scope_var_conversions.wgsl.expected.spvasm b/test/tint/types/module_scope_var_conversions.wgsl.expected.spvasm index 7c8e1425b6..beb15ff3d9 100644 --- a/test/tint/types/module_scope_var_conversions.wgsl.expected.spvasm +++ b/test/tint/types/module_scope_var_conversions.wgsl.expected.spvasm @@ -1,7 +1,7 @@ ; SPIR-V ; Version: 1.3 ; Generator: Google Tint Compiler; 0 -; Bound: 55 +; Bound: 54 ; Schema: 0 OpCapability Shader OpMemoryModel Logical GLSL450 @@ -64,43 +64,42 @@ %v3u32_var1 = OpVariable %_ptr_Private_v3uint Private %32 %v3u32_var2 = OpVariable %_ptr_Private_v3uint Private %32 %v3u32_var3 = OpVariable %_ptr_Private_v3uint Private %32 - %37 = OpConstantComposite %v3bool %true %true %true -%v3bool_var4 = OpVariable %_ptr_Private_v3bool Private %37 +%v3bool_var4 = OpVariable %_ptr_Private_v3bool Private %20 %v4bool = OpTypeVector %bool 4 %false = OpConstantFalse %bool - %41 = OpConstantComposite %v4bool %true %false %true %false + %40 = OpConstantComposite %v4bool %true %false %true %false %_ptr_Private_v4bool = OpTypePointer Private %v4bool -%v4bool_var5 = OpVariable %_ptr_Private_v4bool Private %41 +%v4bool_var5 = OpVariable %_ptr_Private_v4bool Private %40 %void = OpTypeVoid - %44 = OpTypeFunction %void - %48 = OpConstantNull %bool - %49 = OpConstantNull %int - %50 = OpConstantNull %uint - %51 = OpConstantNull %v3bool - %52 = OpConstantNull %v4bool - %53 = OpConstantNull %v3int - %54 = OpConstantNull %v3uint - %main = OpFunction %void None %44 - %47 = OpLabel - OpStore %bool_var1 %48 - OpStore %bool_var2 %48 - OpStore %bool_var3 %48 - OpStore %i32_var1 %49 - OpStore %i32_var2 %49 - OpStore %i32_var3 %49 - OpStore %u32_var1 %50 - OpStore %u32_var2 %50 - OpStore %u32_var3 %50 - OpStore %v3bool_var1 %51 - OpStore %v3bool_var2 %51 - OpStore %v3bool_var3 %51 - OpStore %v3bool_var4 %51 - OpStore %v4bool_var5 %52 - OpStore %v3i32_var1 %53 - OpStore %v3i32_var2 %53 - OpStore %v3i32_var3 %53 - OpStore %v3u32_var1 %54 - OpStore %v3u32_var2 %54 - OpStore %v3u32_var3 %54 + %43 = OpTypeFunction %void + %47 = OpConstantNull %bool + %48 = OpConstantNull %int + %49 = OpConstantNull %uint + %50 = OpConstantNull %v3bool + %51 = OpConstantNull %v4bool + %52 = OpConstantNull %v3int + %53 = OpConstantNull %v3uint + %main = OpFunction %void None %43 + %46 = OpLabel + OpStore %bool_var1 %47 + OpStore %bool_var2 %47 + OpStore %bool_var3 %47 + OpStore %i32_var1 %48 + OpStore %i32_var2 %48 + OpStore %i32_var3 %48 + OpStore %u32_var1 %49 + OpStore %u32_var2 %49 + OpStore %u32_var3 %49 + OpStore %v3bool_var1 %50 + OpStore %v3bool_var2 %50 + OpStore %v3bool_var3 %50 + OpStore %v3bool_var4 %50 + OpStore %v4bool_var5 %51 + OpStore %v3i32_var1 %52 + OpStore %v3i32_var2 %52 + OpStore %v3i32_var3 %52 + OpStore %v3u32_var1 %53 + OpStore %v3u32_var2 %53 + OpStore %v3u32_var3 %53 OpReturn OpFunctionEnd