Remove more uses of AST to_str() and type_name()
These methods are going to be removed as they provide little benefit over the WGSL form, are a maintainance burden and they massively bloat our codebase.
This change introduces sem::CallTargetSignature, which can be used as a std::unordered_map key.
This is used in writer/spirv to replace a map that was keyed off ast::Function::type_name().
Bug: tint:1225
Change-Id: Ic220b3155011f21b14d49eecc8042001148e4ca5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66443
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
diff --git a/src/inspector/inspector.cc b/src/inspector/inspector.cc
index d244c11..d4fa744 100644
--- a/src/inspector/inspector.cc
+++ b/src/inspector/inspector.cc
@@ -806,13 +806,13 @@
continue;
}
- const auto& params = i->Parameters();
- int sampler_index = sem::IndexOf(params, sem::ParameterUsage::kSampler);
+ const auto& signature = i->Signature();
+ int sampler_index = signature.IndexOf(sem::ParameterUsage::kSampler);
if (sampler_index == -1) {
continue;
}
- int texture_index = sem::IndexOf(params, sem::ParameterUsage::kTexture);
+ int texture_index = signature.IndexOf(sem::ParameterUsage::kTexture);
if (texture_index == -1) {
continue;
}
diff --git a/src/inspector/inspector_test.cc b/src/inspector/inspector_test.cc
index 1efef81..bda54c0 100644
--- a/src/inspector/inspector_test.cc
+++ b/src/inspector/inspector_test.cc
@@ -1495,7 +1495,7 @@
// with elem stride of 16, and that is 16-byte aligned within the struct)
ast::Struct* foo_struct_type = Structure(
"foo_type",
- {Member("0__i32", ty.i32()),
+ {Member("0i32", ty.i32()),
Member("b", ty.array(ty.u32(), 4, /*stride*/ 16), {MemberAlign(16)})},
{create<ast::StructBlockDecoration>()});
diff --git a/src/inspector/test_inspector_builder.cc b/src/inspector/test_inspector_builder.cc
index b364218..1d42d58 100644
--- a/src/inspector/test_inspector_builder.cc
+++ b/src/inspector/test_inspector_builder.cc
@@ -84,7 +84,7 @@
}
std::string InspectorBuilder::StructMemberName(size_t idx, ast::Type* type) {
- return std::to_string(idx) + type->type_name();
+ return std::to_string(idx) + type->FriendlyName(Symbols());
}
ast::Struct* InspectorBuilder::MakeStructType(
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index cc64de0..2afd09c 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -2538,8 +2538,8 @@
return false;
}
std::string func_name = intrinsic->str();
- auto index =
- sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kOffset);
+ auto& signature = intrinsic->Signature();
+ auto index = signature.IndexOf(sem::ParameterUsage::kOffset);
if (index > -1) {
auto* param = ast_call->args()[index];
if (param->Is<ast::TypeConstructorExpression>()) {
diff --git a/src/sem/call_target.cc b/src/sem/call_target.cc
index b8bf12c..9ea38bf 100644
--- a/src/sem/call_target.cc
+++ b/src/sem/call_target.cc
@@ -15,6 +15,7 @@
#include "src/sem/call_target.h"
#include "src/symbol_table.h"
+#include "src/utils/hash.h"
TINT_INSTANTIATE_TYPEINFO(tint::sem::CallTarget);
@@ -22,15 +23,20 @@
namespace sem {
CallTarget::CallTarget(sem::Type* return_type, const ParameterList& parameters)
- : return_type_(return_type), parameters_(parameters) {
+ : signature_{return_type, parameters} {
TINT_ASSERT(Semantic, return_type);
}
CallTarget::CallTarget(const CallTarget&) = default;
-
CallTarget::~CallTarget() = default;
-int IndexOf(const ParameterList& parameters, ParameterUsage usage) {
+CallTargetSignature::CallTargetSignature(sem::Type* ret_ty,
+ const ParameterList& params)
+ : return_type(ret_ty), parameters(params) {}
+CallTargetSignature::CallTargetSignature(const CallTargetSignature&) = default;
+CallTargetSignature::~CallTargetSignature() = default;
+
+int CallTargetSignature::IndexOf(ParameterUsage usage) const {
for (size_t i = 0; i < parameters.size(); i++) {
if (parameters[i]->Usage() == usage) {
return static_cast<int>(i);
@@ -39,5 +45,33 @@
return -1;
}
+bool CallTargetSignature::operator==(const CallTargetSignature& other) const {
+ if (return_type != other.return_type ||
+ parameters.size() != other.parameters.size()) {
+ return false;
+ }
+ for (size_t i = 0; i < parameters.size(); i++) {
+ auto* a = parameters[i];
+ auto* b = other.parameters[i];
+ if (a->Type() != b->Type() || a->Usage() != b->Usage()) {
+ return false;
+ }
+ }
+ return true;
+}
+
} // namespace sem
} // namespace tint
+
+namespace std {
+
+std::size_t hash<tint::sem::CallTargetSignature>::operator()(
+ const tint::sem::CallTargetSignature& sig) const {
+ size_t hash = tint::utils::Hash(sig.parameters.size());
+ for (auto* p : sig.parameters) {
+ tint::utils::HashCombine(&hash, p->Type(), p->Usage());
+ }
+ return tint::utils::Hash(hash, sig.return_type);
+}
+
+} // namespace std
diff --git a/src/sem/call_target.h b/src/sem/call_target.h
index 33d5029..c0e7b8e 100644
--- a/src/sem/call_target.h
+++ b/src/sem/call_target.h
@@ -27,11 +27,34 @@
// Forward declarations
class Type;
-/// @param parameters the list of parameters
-/// @param usage the parameter usage to find
-/// @returns the index of the parameter with the given usage, or -1 if no
-/// parameter with the given usage exists.
-int IndexOf(const ParameterList& parameters, ParameterUsage usage);
+/// CallTargetSignature holds the return type and parameters for a call target
+struct CallTargetSignature {
+ /// Constructor
+ /// @param ret_ty the call target return type
+ /// @param params the call target parameters
+ CallTargetSignature(sem::Type* ret_ty, const ParameterList& params);
+
+ /// Copy constructor
+ CallTargetSignature(const CallTargetSignature&);
+
+ /// Destructor
+ ~CallTargetSignature();
+
+ /// The type of the call target return value
+ sem::Type* const return_type = nullptr;
+ /// The parameters of the call target
+ ParameterList const parameters;
+
+ /// Equality operator
+ /// @param other the signature to compare this to
+ /// @returns true if this signature is equal to other
+ bool operator==(const CallTargetSignature& other) const;
+
+ /// @param usage the parameter usage to find
+ /// @returns the index of the parameter with the given usage, or -1 if no
+ /// parameter with the given usage exists.
+ int IndexOf(ParameterUsage usage) const;
+};
/// CallTarget is the base for callable functions
class CallTarget : public Castable<CallTarget, Node> {
@@ -44,21 +67,38 @@
/// Copy constructor
CallTarget(const CallTarget&);
- /// @return the return type of the call target
- sem::Type* ReturnType() const { return return_type_; }
-
/// Destructor
~CallTarget() override;
+ /// @return the return type of the call target
+ sem::Type* ReturnType() const { return signature_.return_type; }
+
/// @return the parameters of the call target
- const ParameterList& Parameters() const { return parameters_; }
+ const ParameterList& Parameters() const { return signature_.parameters; }
+
+ /// @return the signature of the call target
+ const CallTargetSignature& Signature() const { return signature_; }
private:
- sem::Type* const return_type_;
- ParameterList const parameters_;
+ CallTargetSignature signature_;
};
} // namespace sem
} // namespace tint
+namespace std {
+
+/// Custom std::hash specialization for tint::sem::CallTargetSignature so
+/// CallTargetSignature can be used as keys for std::unordered_map and
+/// std::unordered_set.
+template <>
+class hash<tint::sem::CallTargetSignature> {
+ public:
+ /// @param sig the CallTargetSignature to hash
+ /// @return the hash value
+ std::size_t operator()(const tint::sem::CallTargetSignature& sig) const;
+};
+
+} // namespace std
+
#endif // SRC_SEM_CALL_TARGET_H_
diff --git a/src/transform/robustness.cc b/src/transform/robustness.cc
index b6a111e..a5f1eac 100644
--- a/src/transform/robustness.cc
+++ b/src/transform/robustness.cc
@@ -208,14 +208,11 @@
// Indices of the mandatory texture and coords parameters, and the optional
// array and level parameters.
- auto texture_idx =
- sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kTexture);
- auto coords_idx =
- sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kCoords);
- auto array_idx =
- sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kArrayIndex);
- auto level_idx =
- sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kLevel);
+ auto& signature = intrinsic->Signature();
+ auto texture_idx = signature.IndexOf(sem::ParameterUsage::kTexture);
+ auto coords_idx = signature.IndexOf(sem::ParameterUsage::kCoords);
+ auto array_idx = signature.IndexOf(sem::ParameterUsage::kArrayIndex);
+ auto level_idx = signature.IndexOf(sem::ParameterUsage::kLevel);
auto* texture_arg = expr->args()[texture_idx];
auto* coords_arg = expr->args()[coords_idx];
diff --git a/src/writer/glsl/generator_impl.cc b/src/writer/glsl/generator_impl.cc
index 91fb6dc..54aa31e 100644
--- a/src/writer/glsl/generator_impl.cc
+++ b/src/writer/glsl/generator_impl.cc
@@ -960,12 +960,12 @@
const sem::Intrinsic* intrinsic) {
using Usage = sem::ParameterUsage;
- auto parameters = intrinsic->Parameters();
+ auto& signature = intrinsic->Signature();
auto arguments = expr->args();
// Returns the argument with the given usage
auto arg = [&](Usage usage) {
- int idx = sem::IndexOf(parameters, usage);
+ int idx = signature.IndexOf(usage);
return (idx >= 0) ? arguments[idx] : nullptr;
};
@@ -1395,8 +1395,9 @@
return EmitUnaryOp(out, u);
}
- diagnostics_.add_error(diag::System::Writer,
- "unknown expression type: " + builder_.str(expr));
+ diagnostics_.add_error(
+ diag::System::Writer,
+ "unknown expression type: " + std::string(expr->TypeInfo().name));
return false;
}
@@ -2235,8 +2236,9 @@
return EmitVariable(v->variable());
}
- diagnostics_.add_error(diag::System::Writer,
- "unknown statement type: " + builder_.str(stmt));
+ diagnostics_.add_error(
+ diag::System::Writer,
+ "unknown statement type: " + std::string(stmt->TypeInfo().name));
return false;
}
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc
index 7301166..90eeaa0 100644
--- a/src/writer/hlsl/generator_impl.cc
+++ b/src/writer/hlsl/generator_impl.cc
@@ -1620,12 +1620,12 @@
const sem::Intrinsic* intrinsic) {
using Usage = sem::ParameterUsage;
- auto parameters = intrinsic->Parameters();
+ auto& signature = intrinsic->Signature();
auto arguments = expr->args();
// Returns the argument with the given usage
auto arg = [&](Usage usage) {
- int idx = sem::IndexOf(parameters, usage);
+ int idx = signature.IndexOf(usage);
return (idx >= 0) ? arguments[idx] : nullptr;
};
@@ -2215,8 +2215,9 @@
return EmitUnaryOp(out, u);
}
- diagnostics_.add_error(diag::System::Writer,
- "unknown expression type: " + builder_.str(expr));
+ diagnostics_.add_error(
+ diag::System::Writer,
+ "unknown expression type: " + std::string(expr->TypeInfo().name));
return false;
}
@@ -2934,8 +2935,9 @@
return EmitVariable(v->variable());
}
- diagnostics_.add_error(diag::System::Writer,
- "unknown statement type: " + builder_.str(stmt));
+ diagnostics_.add_error(
+ diag::System::Writer,
+ "unknown statement type: " + std::string(stmt->TypeInfo().name));
return false;
}
diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc
index 6c839fb..89e8d06 100644
--- a/src/writer/msl/generator_impl.cc
+++ b/src/writer/msl/generator_impl.cc
@@ -764,12 +764,12 @@
const sem::Intrinsic* intrinsic) {
using Usage = sem::ParameterUsage;
- auto parameters = intrinsic->Parameters();
+ auto& signature = intrinsic->Signature();
auto arguments = expr->args();
// Returns the argument with the given usage
auto arg = [&](Usage usage) {
- int idx = sem::IndexOf(parameters, usage);
+ int idx = signature.IndexOf(usage);
return (idx >= 0) ? arguments[idx] : nullptr;
};
@@ -1427,8 +1427,9 @@
return EmitUnaryOp(out, u);
}
- diagnostics_.add_error(diag::System::Writer,
- "unknown expression type: " + program_->str(expr));
+ diagnostics_.add_error(
+ diag::System::Writer,
+ "unknown expression type: " + std::string(expr->TypeInfo().name));
return false;
}
@@ -2052,8 +2053,9 @@
return EmitVariable(var);
}
- diagnostics_.add_error(diag::System::Writer,
- "unknown statement type: " + program_->str(stmt));
+ diagnostics_.add_error(
+ diag::System::Writer,
+ "unknown statement type: " + std::string(stmt->TypeInfo().name));
return false;
}
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index 23be508..0de0335 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -584,7 +584,7 @@
return GenerateUnaryOpExpression(u);
}
- error_ = "unknown expression type: " + builder_.str(expr);
+ error_ = "unknown expression type: " + std::string(expr->TypeInfo().name);
return 0;
}
@@ -659,32 +659,28 @@
}
uint32_t Builder::GenerateFunctionTypeIfNeeded(const sem::Function* func) {
- auto val = type_name_to_id_.find(func->Declaration()->type_name());
- if (val != type_name_to_id_.end()) {
- return val->second;
- }
+ return utils::GetOrCreate(
+ func_sig_to_id_, func->Signature(), [&]() -> uint32_t {
+ auto func_op = result_op();
+ auto func_type_id = func_op.to_i();
- auto func_op = result_op();
- auto func_type_id = func_op.to_i();
+ auto ret_id = GenerateTypeIfNeeded(func->ReturnType());
+ if (ret_id == 0) {
+ return 0;
+ }
- auto ret_id = GenerateTypeIfNeeded(func->ReturnType());
- if (ret_id == 0) {
- return 0;
- }
+ OperandList ops = {func_op, Operand::Int(ret_id)};
+ for (auto* param : func->Parameters()) {
+ auto param_type_id = GenerateTypeIfNeeded(param->Type());
+ if (param_type_id == 0) {
+ return 0;
+ }
+ ops.push_back(Operand::Int(param_type_id));
+ }
- OperandList ops = {func_op, Operand::Int(ret_id)};
- for (auto* param : func->Parameters()) {
- auto param_type_id = GenerateTypeIfNeeded(param->Type());
- if (param_type_id == 0) {
- return 0;
- }
- ops.push_back(Operand::Int(param_type_id));
- }
-
- push_type(spv::Op::OpTypeFunction, std::move(ops));
-
- type_name_to_id_[func->Declaration()->type_name()] = func_type_id;
- return func_type_id;
+ push_type(spv::Op::OpTypeFunction, std::move(ops));
+ return func_type_id;
+ });
}
bool Builder::GenerateFunctionVariable(ast::Variable* var) {
@@ -1139,7 +1135,8 @@
}
} else {
- error_ = "invalid accessor in list: " + builder_.str(accessor);
+ error_ =
+ "invalid accessor in list: " + std::string(accessor->TypeInfo().name);
return 0;
}
}
@@ -2662,7 +2659,7 @@
Operand result_id) {
using Usage = sem::ParameterUsage;
- auto parameters = intrinsic->Parameters();
+ auto& signature = intrinsic->Signature();
auto arguments = call->args();
// Generates the given expression, returning the operand ID
@@ -2678,7 +2675,7 @@
// Returns the argument with the given usage
auto arg = [&](Usage usage) {
- int idx = sem::IndexOf(parameters, usage);
+ int idx = signature.IndexOf(usage);
return (idx >= 0) ? arguments[idx] : nullptr;
};
@@ -3785,7 +3782,7 @@
return GenerateVariableDeclStatement(v);
}
- error_ = "Unknown statement: " + builder_.str(stmt);
+ error_ = "Unknown statement: " + std::string(stmt->TypeInfo().name);
return false;
}
diff --git a/src/writer/spirv/builder.h b/src/writer/spirv/builder.h
index 8af4cf2..baa4d7f 100644
--- a/src/writer/spirv/builder.h
+++ b/src/writer/spirv/builder.h
@@ -585,6 +585,7 @@
std::unordered_map<std::string, uint32_t> import_name_to_id_;
std::unordered_map<Symbol, uint32_t> func_symbol_to_id_;
+ std::unordered_map<sem::CallTargetSignature, uint32_t> func_sig_to_id_;
std::unordered_map<std::string, uint32_t> type_name_to_id_;
std::unordered_map<ScalarConstant, uint32_t> const_to_id_;
std::unordered_map<std::string, uint32_t> type_constructor_to_id_;
diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc
index 1d12bcb..c226b1f 100644
--- a/src/writer/wgsl/generator_impl.cc
+++ b/src/writer/wgsl/generator_impl.cc
@@ -524,8 +524,9 @@
} else if (auto* tn = ty->As<ast::TypeName>()) {
out << program_->Symbols().NameFor(tn->name());
} else {
- diagnostics_.add_error(diag::System::Writer,
- "unknown type in EmitType: " + ty->type_name());
+ diagnostics_.add_error(
+ diag::System::Writer,
+ "unknown type in EmitType: " + std::string(ty->TypeInfo().name));
return false;
}
return true;
@@ -869,8 +870,9 @@
return EmitVariable(line(), v->variable());
}
- diagnostics_.add_error(diag::System::Writer,
- "unknown statement type: " + program_->str(stmt));
+ diagnostics_.add_error(
+ diag::System::Writer,
+ "unknown statement type: " + std::string(stmt->TypeInfo().name));
return false;
}
