[ir_to_program] Cleanup IRToProgram.
This Cl simplifies the binding tracking to just store the `ir_expr`,
`ast_expr` or `name` we care about and drops the use of the `variant`
and visitor code. The `SCOPED_NESTING` is removed in favour of the more
commonly used `TINT_SCOPED_ASSIGNMENT`.
Bug: 422532802
Change-Id: I6b3aa199276c26069f229b4fa6f17352859ec280
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/245614
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
diff --git a/src/tint/lang/wgsl/writer/ir_to_program/ir_to_program.cc b/src/tint/lang/wgsl/writer/ir_to_program/ir_to_program.cc
index b0a5a7a..333781c 100644
--- a/src/tint/lang/wgsl/writer/ir_to_program/ir_to_program.cc
+++ b/src/tint/lang/wgsl/writer/ir_to_program/ir_to_program.cc
@@ -97,12 +97,6 @@
#include "src/tint/utils/math/math.h"
#include "src/tint/utils/rtti/switch.h"
-// Helper for incrementing nesting_depth_ and then decrementing nesting_depth_ at the end
-// of the scope that holds the call.
-#define SCOPED_NESTING() \
- nesting_depth_++; \
- TINT_DEFER(nesting_depth_--)
-
using namespace tint::core::fluent_types; // NOLINT
namespace tint::wgsl::writer {
@@ -153,28 +147,13 @@
/// The target ProgramBuilder
ProgramBuilder b;
- /// The structure for a value held by a 'let', 'var' or parameter.
- struct VariableValue {
- Symbol name; // Name of the variable
- };
-
/// The structure for a reusable value
- struct ReusableValue {
- const core::ir::Value* expr = nullptr;
+ struct ValueBinding {
+ Symbol name{};
+ const core::ir::Value* ir_expr = nullptr;
+ const ast::Expression* ast_expr = nullptr;
};
- /// The structure for an inlined value
- struct InlinedValue {
- const ast::Expression* expr = nullptr;
- };
-
- /// Empty struct used as a sentinel value to indicate that an ast::Value has been consumed by
- /// its single place of usage. Attempting to use this value a second time should result in an
- /// ICE.
- struct ConsumedValue {};
-
- using ValueBinding = std::variant<VariableValue, InlinedValue, ConsumedValue, ReusableValue>;
-
/// IR values to their representation
Hashmap<const core::ir::Value*, ValueBinding, 32> bindings_;
@@ -214,7 +193,7 @@
}
}
const ast::Function* Fn(const core::ir::Function* fn) {
- SCOPED_NESTING();
+ TINT_SCOPED_ASSIGNMENT(nesting_depth_, nesting_depth_ + 1);
// Emit parameters.
static constexpr size_t N = decltype(ast::Function::params)::static_length;
@@ -395,7 +374,7 @@
}
void If(const core::ir::If* if_) {
- SCOPED_NESTING();
+ TINT_SCOPED_ASSIGNMENT(nesting_depth_, nesting_depth_ + 1);
auto true_stmts = Statements(if_->True());
auto false_stmts = Statements(if_->False());
@@ -423,7 +402,7 @@
}
void Loop(const core::ir::Loop* l) {
- SCOPED_NESTING();
+ TINT_SCOPED_ASSIGNMENT(nesting_depth_, nesting_depth_ + 1);
// Build all the initializer statements
auto init_stmts = Statements(l->Initializer());
@@ -484,9 +463,8 @@
// Depending on 'init', 'cond' and 'cont', build a 'for', 'while' or 'loop'
const ast::Statement* loop = nullptr;
- if ((!cont && !cont_stmts.IsEmpty()) // Non-trivial continuing
- || !cond // or non-trivial or no condition
- ) {
+ bool non_trivial_continuing = !cont && !cont_stmts.IsEmpty();
+ if (non_trivial_continuing || !cond) {
// Build a loop
if (cond) {
body_stmts.Insert(0, b.If(b.Not(cond), b.Block(b.Break())));
@@ -519,14 +497,14 @@
}
void Switch(const core::ir::Switch* s) {
- SCOPED_NESTING();
+ TINT_SCOPED_ASSIGNMENT(nesting_depth_, nesting_depth_ + 1);
auto* cond = Expr(s->Condition());
auto cases = tint::Transform<4>(
s->Cases(), //
[&](const core::ir::Switch::Case& c) -> const tint::ast::CaseStatement* {
- SCOPED_NESTING();
+ TINT_SCOPED_ASSIGNMENT(nesting_depth_, nesting_depth_ + 1);
const ast::BlockStatement* body = nullptr;
{
@@ -839,7 +817,7 @@
auto* vec = Expr(s->Object());
Vector<char, 4> components;
for (uint32_t i : s->Indices()) {
- if (DAWN_UNLIKELY(i >= 4)) {
+ if (i >= 4) {
TINT_ICE() << "invalid swizzle index: " << i;
}
components.Push(xyzw[i]);
@@ -921,57 +899,25 @@
Bind(e->Result(), expr);
}
- TINT_BEGIN_DISABLE_WARNING(UNREACHABLE_CODE);
-
const ast::Expression* Expr(const core::ir::Value* value) {
- auto expr = tint::Switch(
- value, //
- [&](const core::ir::Constant* c) { return Constant(c); },
- [&](Default) -> const ast::Expression* {
- auto lookup = bindings_.Get(value);
- if (DAWN_UNLIKELY(!lookup)) {
- TINT_ICE() << "Expr(" << (value ? value->TypeInfo().name : "null")
- << ") value has no expression";
- }
- return std::visit(
- [&](auto&& got) -> const ast::Expression* {
- using T = std::decay_t<decltype(got)>;
-
- if constexpr (std::is_same_v<T, VariableValue>) {
- return b.Expr(got.name);
- }
- if constexpr (std::is_same_v<T, ReusableValue>) {
- return Expr(got.expr);
- }
-
- if constexpr (std::is_same_v<T, InlinedValue>) {
- auto result = got.expr;
- // Single use (inlined) expression.
- // Mark the bindings_ map entry as consumed.
- *lookup = ConsumedValue{};
- return result;
- }
-
- if constexpr (std::is_same_v<T, ConsumedValue>) {
- TINT_ICE() << "Expr(" << value->TypeInfo().name
- << ") called twice on the same value";
- } else {
- TINT_ICE()
- << "Expr(" << value->TypeInfo().name << ") has unhandled value";
- }
- return nullptr;
- },
- *lookup);
- });
-
- if (!expr) {
- return b.Expr("<error>");
+ if (auto* cnst = value->As<core::ir::Constant>()) {
+ return Constant(cnst);
}
- return expr;
- }
+ auto lookup = bindings_.Get(value);
+ if (!lookup) {
+ TINT_ICE() << "Expr(" << (value ? value->TypeInfo().name : "null")
+ << ") value has no expression";
+ }
- TINT_END_DISABLE_WARNING(UNREACHABLE_CODE);
+ if (lookup->ast_expr != nullptr) {
+ return lookup->ast_expr;
+ }
+ if (lookup->ir_expr != nullptr) {
+ return Expr(lookup->ir_expr);
+ }
+ return b.Expr(lookup->name);
+ }
const ast::Expression* Constant(const core::ir::Constant* c) { return Constant(c->Value()); }
@@ -1014,16 +960,6 @@
}
}
- ////////////////////////////////////////////////////////////////////////////////////////////////
- // Types
- //
- // The the case of an error:
- // * The types generating methods must return a non-null ast type, which may not be semantically
- // legal, but is enough to populate the AST.
- // * A diagnostic error must be added to the ast::ProgramBuilder.
- // This prevents littering the ToProgram logic with expensive error checking code.
- ////////////////////////////////////////////////////////////////////////////////////////////////
-
/// @param ty the type::Type
/// @return an ast::Type from @p ty.
/// @note May be a semantically-invalid placeholder type on error.
@@ -1061,7 +997,7 @@
return b.ty.array(el, std::move(attrs));
}
auto count = a->ConstantCount();
- if (DAWN_UNLIKELY(!count)) {
+ if (!count) {
TINT_ICE() << core::type::Array::kErrExpectedConstantCount;
}
return b.ty.array(el, u32(count.value()), std::move(attrs));
@@ -1253,7 +1189,7 @@
void Bind(const core::ir::Value* value, const core::ir::Value* expr) {
TINT_ASSERT(value);
if (value->IsUsed()) {
- bindings_.Replace(value, ReusableValue{expr});
+ bindings_.Replace(value, ValueBinding{.ir_expr = expr});
} else {
Append(b.Assign(b.Phony(), Expr(expr)));
}
@@ -1264,8 +1200,7 @@
void Bind(const core::ir::Value* value, const ast::Expression* expr) {
TINT_ASSERT(value);
if (value->IsUsed()) {
- // Value will be inlined at its place of usage.
- if (DAWN_UNLIKELY(!bindings_.Add(value, InlinedValue{expr}))) {
+ if (!bindings_.Add(value, ValueBinding{.ast_expr = expr})) {
TINT_ICE() << "Bind(" << value->TypeInfo().name << ") called twice for same value";
}
} else {
@@ -1277,7 +1212,7 @@
/// name.
void Bind(const core::ir::Value* value, Symbol name) {
TINT_ASSERT(value);
- if (DAWN_UNLIKELY(!bindings_.Add(value, VariableValue{name}))) {
+ if (!bindings_.Add(value, ValueBinding{.name = name})) {
TINT_ICE() << "Bind(" << value->TypeInfo().name << ") called twice for same value";
}
}
