transform: Reimplement FirstIndexOffset
* It didn't always produce valid WGSL (crbug.com/tint/687)
* It didn't handle builtins as entry point parameters
* It used hard-coded symbols that could collide
* It didn't use DataMap for input.
The new implementation addresses all of this.
Bug: tint:687
Change-Id: I447bec530b45414ebb8baeb4ee18261d73d1c0d2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47222
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
diff --git a/src/transform/first_index_offset.cc b/src/transform/first_index_offset.cc
index 3766e01..a1adf66 100644
--- a/src/transform/first_index_offset.cc
+++ b/src/transform/first_index_offset.cc
@@ -15,40 +15,34 @@
#include "src/transform/first_index_offset.h"
#include <memory>
+#include <unordered_map>
#include <utility>
#include "src/ast/struct_block_decoration.h"
#include "src/program_builder.h"
#include "src/semantic/function.h"
+#include "src/semantic/member_accessor_expression.h"
+#include "src/semantic/struct.h"
+#include "src/semantic/variable.h"
+TINT_INSTANTIATE_TYPEINFO(tint::transform::FirstIndexOffset::BindingPoint);
TINT_INSTANTIATE_TYPEINFO(tint::transform::FirstIndexOffset::Data);
namespace tint {
namespace transform {
namespace {
-constexpr char kStructName[] = "TintFirstIndexOffsetData";
-constexpr char kBufferName[] = "tint_first_index_data";
-constexpr char kFirstVertexName[] = "tint_first_vertex_index";
-constexpr char kFirstInstanceName[] = "tint_first_instance_index";
-constexpr char kIndexOffsetPrefix[] = "tint_first_index_offset_";
-
-ast::Variable* clone_variable_with_new_name(CloneContext* ctx,
- ast::Variable* in,
- std::string new_name) {
- // Clone arguments outside of create() call to have deterministic ordering
- auto source = ctx->Clone(in->source());
- auto symbol = ctx->dst->Symbols().Register(new_name);
- auto* type = ctx->Clone(in->declared_type());
- auto* constructor = ctx->Clone(in->constructor());
- auto decorations = ctx->Clone(in->decorations());
- return ctx->dst->create<ast::Variable>(
- source, symbol, in->declared_storage_class(), type, in->is_const(),
- constructor, decorations);
-}
+// Uniform buffer member names
+constexpr char kFirstVertexName[] = "first_vertex_index";
+constexpr char kFirstInstanceName[] = "first_instance_index";
} // namespace
+FirstIndexOffset::BindingPoint::BindingPoint() = default;
+FirstIndexOffset::BindingPoint::BindingPoint(uint32_t b, uint32_t g)
+ : binding(b), group(g) {}
+FirstIndexOffset::BindingPoint::~BindingPoint() = default;
+
FirstIndexOffset::Data::Data(bool has_vtx_index,
bool has_inst_index,
uint32_t first_vtx_offset,
@@ -57,172 +51,134 @@
has_instance_index(has_inst_index),
first_vertex_offset(first_vtx_offset),
first_instance_offset(first_inst_offset) {}
-
FirstIndexOffset::Data::Data(const Data&) = default;
-
FirstIndexOffset::Data::~Data() = default;
+FirstIndexOffset::FirstIndexOffset() = default;
FirstIndexOffset::FirstIndexOffset(uint32_t binding, uint32_t group)
: binding_(binding), group_(group) {}
FirstIndexOffset::~FirstIndexOffset() = default;
-Transform::Output FirstIndexOffset::Run(const Program* in, const DataMap&) {
- ProgramBuilder out;
-
- // First do a quick check to see if the transform has already been applied.
- for (ast::Variable* var : in->AST().GlobalVariables()) {
- if (auto* dec_var = var->As<ast::Variable>()) {
- if (dec_var->symbol() == in->Symbols().Get(kBufferName)) {
- out.Diagnostics().add_error(
- "First index offset transform has already been applied.");
- return Output(Program(std::move(out)));
- }
- }
+Transform::Output FirstIndexOffset::Run(const Program* in,
+ const DataMap& data) {
+ // Get the uniform buffer binding point
+ uint32_t ub_binding = binding_;
+ uint32_t ub_group = group_;
+ if (auto* binding_point = data.Get<BindingPoint>()) {
+ ub_binding = binding_point->binding;
+ ub_group = binding_point->group;
}
- State state{&out, binding_, group_};
-
- Symbol vertex_index_sym;
- Symbol instance_index_sym;
-
- // Lazily construct the UniformBuffer on first call to
- // maybe_create_buffer_var()
- ast::Variable* buffer_var = nullptr;
- auto maybe_create_buffer_var = [&]() {
- if (buffer_var == nullptr) {
- buffer_var = state.AddUniformBuffer();
- }
- };
-
- // Clone the AST, renaming the kVertexIndex and kInstanceIndex builtins, and
- // add a CreateFirstIndexOffset() statement to each function that uses one of
- // these builtins.
-
+ ProgramBuilder out;
CloneContext ctx(&out, in);
- ctx.ReplaceAll([&](ast::Variable* var) -> ast::Variable* {
- for (ast::Decoration* dec : var->decorations()) {
- if (auto* blt_dec = dec->As<ast::BuiltinDecoration>()) {
- ast::Builtin blt_type = blt_dec->value();
- if (blt_type == ast::Builtin::kVertexIndex) {
- vertex_index_sym = var->symbol();
- state.has_vertex_index = true;
- return clone_variable_with_new_name(
- &ctx, var,
- kIndexOffsetPrefix + in->Symbols().NameFor(var->symbol()));
- } else if (blt_type == ast::Builtin::kInstanceIndex) {
- instance_index_sym = var->symbol();
- state.has_instance_index = true;
- return clone_variable_with_new_name(
- &ctx, var,
- kIndexOffsetPrefix + in->Symbols().NameFor(var->symbol()));
- }
- }
- }
- return nullptr; // Just clone var
- });
- ctx.ReplaceAll( // Note: This happens in the same pass as the rename above
- // which determines the original builtin variable names,
- // but this should be fine, as variables are cloned first.
- [&](ast::Function* func) -> ast::Function* {
- maybe_create_buffer_var();
- if (buffer_var == nullptr) {
- return nullptr; // no transform need, just clone func
- }
- auto* func_sem = in->Sem().Get(func);
- ast::StatementList statements;
- for (const auto& data : func_sem->LocalReferencedBuiltinVariables()) {
- if (data.second->value() == ast::Builtin::kVertexIndex) {
- statements.emplace_back(state.CreateFirstIndexOffset(
- in->Symbols().NameFor(vertex_index_sym), kFirstVertexName,
- buffer_var));
- } else if (data.second->value() == ast::Builtin::kInstanceIndex) {
- statements.emplace_back(state.CreateFirstIndexOffset(
- in->Symbols().NameFor(instance_index_sym), kFirstInstanceName,
- buffer_var));
+
+ // Map of builtin usages
+ std::unordered_map<const semantic::Variable*, const char*> builtin_vars;
+ std::unordered_map<const semantic::StructMember*, const char*>
+ builtin_members;
+
+ bool has_vertex_index = false;
+ bool has_instance_index = false;
+
+ // Traverse the AST scanning for builtin accesses via variables (includes
+ // parameters) or structure member accesses.
+ for (auto* node : in->ASTNodes().Objects()) {
+ if (auto* var = node->As<ast::Variable>()) {
+ for (ast::Decoration* dec : var->decorations()) {
+ if (auto* builtin_dec = dec->As<ast::BuiltinDecoration>()) {
+ ast::Builtin builtin = builtin_dec->value();
+ if (builtin == ast::Builtin::kVertexIndex) {
+ auto* sem_var = ctx.src->Sem().Get(var);
+ builtin_vars.emplace(sem_var, kFirstVertexName);
+ has_vertex_index = true;
+ }
+ if (builtin == ast::Builtin::kInstanceIndex) {
+ auto* sem_var = ctx.src->Sem().Get(var);
+ builtin_vars.emplace(sem_var, kFirstInstanceName);
+ has_instance_index = true;
}
}
- return CloneWithStatementsAtStart(&ctx, func, statements);
- });
+ }
+ }
+ if (auto* member = node->As<ast::StructMember>()) {
+ for (ast::Decoration* dec : member->decorations()) {
+ if (auto* builtin_dec = dec->As<ast::BuiltinDecoration>()) {
+ ast::Builtin builtin = builtin_dec->value();
+ if (builtin == ast::Builtin::kVertexIndex) {
+ auto* sem_mem = ctx.src->Sem().Get(member);
+ builtin_members.emplace(sem_mem, kFirstVertexName);
+ has_vertex_index = true;
+ }
+ if (builtin == ast::Builtin::kInstanceIndex) {
+ auto* sem_mem = ctx.src->Sem().Get(member);
+ builtin_members.emplace(sem_mem, kFirstInstanceName);
+ has_instance_index = true;
+ }
+ }
+ }
+ }
+ }
+
+ // Byte offsets on the uniform buffer
+ uint32_t vertex_index_offset = 0;
+ uint32_t instance_index_offset = 0;
+
+ if (has_vertex_index || has_instance_index) {
+ // Add uniform buffer members and calculate byte offsets
+ uint32_t offset = 0;
+ ast::StructMemberList members;
+ if (has_vertex_index) {
+ members.push_back(ctx.dst->Member(kFirstVertexName, ctx.dst->ty.u32()));
+ vertex_index_offset = offset;
+ offset += 4;
+ }
+ if (has_instance_index) {
+ members.push_back(ctx.dst->Member(kFirstInstanceName, ctx.dst->ty.u32()));
+ instance_index_offset = offset;
+ offset += 4;
+ }
+ auto* struct_type =
+ ctx.dst->Structure(ctx.dst->Symbols().New(), std::move(members),
+ {ctx.dst->create<ast::StructBlockDecoration>()});
+
+ // Create a global to hold the uniform buffer
+ Symbol buffer_name = ctx.dst->Symbols().New();
+ ctx.dst->Global(buffer_name, struct_type, ast::StorageClass::kUniform,
+ nullptr,
+ ast::DecorationList{
+ ctx.dst->create<ast::BindingDecoration>(ub_binding),
+ ctx.dst->create<ast::GroupDecoration>(ub_group),
+ });
+
+ // Fix up all references to the builtins with the offsets
+ ctx.ReplaceAll([=, &ctx](ast::Expression* expr) -> ast::Expression* {
+ auto* sem = ctx.src->Sem().Get(expr);
+ if (auto* user = sem->As<semantic::VariableUser>()) {
+ auto it = builtin_vars.find(user->Variable());
+ if (it != builtin_vars.end()) {
+ return ctx.dst->Add(ctx.CloneWithoutTransform(expr),
+ ctx.dst->MemberAccessor(buffer_name, it->second));
+ }
+ }
+ if (auto* access = sem->As<semantic::StructMemberAccess>()) {
+ auto it = builtin_members.find(access->Member());
+ if (it != builtin_members.end()) {
+ return ctx.dst->Add(ctx.CloneWithoutTransform(expr),
+ ctx.dst->MemberAccessor(buffer_name, it->second));
+ }
+ }
+ // Not interested in this experssion. Just clone.
+ return nullptr;
+ });
+ }
+
ctx.Clone();
- return Output(Program(std::move(out)),
- std::make_unique<Data>(
- state.has_vertex_index, state.has_instance_index,
- state.vertex_index_offset, state.instance_index_offset));
-}
-
-ast::Variable* FirstIndexOffset::State::AddUniformBuffer() {
- auto* u32_type = dst->create<type::U32>();
- uint32_t offset = 0;
- ast::StructMemberList members;
- if (has_vertex_index) {
- members.push_back(dst->create<ast::StructMember>(
- Source{}, dst->Symbols().Register(kFirstVertexName), u32_type,
- ast::DecorationList{}));
- vertex_index_offset = offset;
- offset += 4;
- }
-
- if (has_instance_index) {
- members.push_back(dst->create<ast::StructMember>(
- Source{}, dst->Symbols().Register(kFirstInstanceName), u32_type,
- ast::DecorationList{}));
- instance_index_offset = offset;
- offset += 4;
- }
-
- ast::DecorationList decos;
- decos.push_back(dst->create<ast::StructBlockDecoration>(Source{}));
-
- auto* struct_type = dst->create<type::Struct>(
- dst->Symbols().Register(kStructName),
- dst->create<ast::Struct>(Source{}, std::move(members), std::move(decos)));
-
- auto* idx_var = dst->create<ast::Variable>(
- Source{}, // source
- dst->Symbols().Register(kBufferName), // symbol
- ast::StorageClass::kUniform, // storage_class
- struct_type, // type
- false, // is_const
- nullptr, // constructor
- ast::DecorationList{
- dst->create<ast::BindingDecoration>(Source{}, binding),
- dst->create<ast::GroupDecoration>(Source{}, group),
- });
-
- dst->AST().AddConstructedType(struct_type);
-
- dst->AST().AddGlobalVariable(idx_var);
-
- return idx_var;
-}
-
-ast::VariableDeclStatement* FirstIndexOffset::State::CreateFirstIndexOffset(
- const std::string& original_name,
- const std::string& field_name,
- ast::Variable* buffer_var) {
- auto* buffer =
- dst->create<ast::IdentifierExpression>(Source{}, buffer_var->symbol());
-
- auto lhs_name = kIndexOffsetPrefix + original_name;
- auto* constructor = dst->create<ast::BinaryExpression>(
- Source{}, ast::BinaryOp::kAdd,
- dst->create<ast::IdentifierExpression>(Source{},
- dst->Symbols().Register(lhs_name)),
- dst->create<ast::MemberAccessorExpression>(
- Source{}, buffer,
- dst->create<ast::IdentifierExpression>(
- Source{}, dst->Symbols().Register(field_name))));
- auto* var = dst->create<ast::Variable>(
- Source{}, // source
- dst->Symbols().Register(original_name), // symbol
- ast::StorageClass::kNone, // storage_class
- dst->create<type::U32>(), // type
- true, // is_const
- constructor, // constructor
- ast::DecorationList{}); // decorations
- return dst->create<ast::VariableDeclStatement>(Source{}, var);
+ return Output(
+ Program(std::move(out)),
+ std::make_unique<Data>(has_vertex_index, has_instance_index,
+ vertex_index_offset, instance_index_offset));
}
} // namespace transform
