resolver: Allocate IDs for named pipeline constants
Keep track of any constant IDs specified in the shader, and then
allocate IDs for the remaining constants when creating the semantic
info.
Bug: tint:755
Change-Id: I6a76b1193cac459b62582cde7469b092dde51d5d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50841
Commit-Queue: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 2bd1b09..6e5f3ab 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -550,6 +550,7 @@
resolver/intrinsic_test.cc
resolver/is_host_shareable_test.cc
resolver/is_storeable_test.cc
+ resolver/pipeline_overridable_constant_test.cc
resolver/resolver_test_helper.cc
resolver/resolver_test_helper.h
resolver/resolver_test.cc
diff --git a/src/program_builder.h b/src/program_builder.h
index 7c8d5ba..833cb33 100644
--- a/src/program_builder.h
+++ b/src/program_builder.h
@@ -38,6 +38,7 @@
#include "src/ast/member_accessor_expression.h"
#include "src/ast/module.h"
#include "src/ast/multisampled_texture.h"
+#include "src/ast/override_decoration.h"
#include "src/ast/pointer.h"
#include "src/ast/return_statement.h"
#include "src/ast/sampled_texture.h"
@@ -1786,6 +1787,32 @@
return create<ast::LocationDecoration>(source_, location);
}
+ /// Creates an ast::OverrideDecoration with a specific constant ID
+ /// @param source the source information
+ /// @param id the id value
+ /// @returns the override decoration pointer
+ ast::OverrideDecoration* Override(const Source& source, uint32_t id) {
+ return create<ast::OverrideDecoration>(source, id);
+ }
+
+ /// Creates an ast::OverrideDecoration with a specific constant ID
+ /// @param id the optional id value
+ /// @returns the override decoration pointer
+ ast::OverrideDecoration* Override(uint32_t id) {
+ return Override(source_, id);
+ }
+
+ /// Creates an ast::OverrideDecoration without a constant ID
+ /// @param source the source information
+ /// @returns the override decoration pointer
+ ast::OverrideDecoration* Override(const Source& source) {
+ return create<ast::OverrideDecoration>(source);
+ }
+
+ /// Creates an ast::OverrideDecoration without a constant ID
+ /// @returns the override decoration pointer
+ ast::OverrideDecoration* Override() { return Override(source_); }
+
/// Creates an ast::StageDecoration
/// @param source the source information
/// @param stage the pipeline stage
diff --git a/src/resolver/pipeline_overridable_constant_test.cc b/src/resolver/pipeline_overridable_constant_test.cc
new file mode 100644
index 0000000..48a3456
--- /dev/null
+++ b/src/resolver/pipeline_overridable_constant_test.cc
@@ -0,0 +1,113 @@
+// Copyright 2021 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/resolver/resolver.h"
+
+#include "gmock/gmock.h"
+#include "src/resolver/resolver_test_helper.h"
+
+using ::testing::UnorderedElementsAre;
+
+namespace tint {
+namespace resolver {
+namespace {
+
+using ResolverPipelineOverridableConstantTest = ResolverTest;
+
+TEST_F(ResolverPipelineOverridableConstantTest, NonOverridable) {
+ auto* a = GlobalConst("a", ty.f32(), Construct(ty.f32()));
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+
+ auto* sem_a = Sem().Get(a);
+ ASSERT_NE(sem_a, nullptr);
+ EXPECT_EQ(sem_a->Declaration(), a);
+ EXPECT_FALSE(sem_a->IsPipelineConstant());
+}
+
+TEST_F(ResolverPipelineOverridableConstantTest, WithId) {
+ auto* a = GlobalConst("a", ty.f32(), Construct(ty.f32()), {Override(7u)});
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+
+ auto* sem_a = Sem().Get(a);
+ ASSERT_NE(sem_a, nullptr);
+ EXPECT_EQ(sem_a->Declaration(), a);
+ EXPECT_TRUE(sem_a->IsPipelineConstant());
+ EXPECT_EQ(sem_a->ConstantId(), 7u);
+}
+
+TEST_F(ResolverPipelineOverridableConstantTest, WithoutId) {
+ auto* a = GlobalConst("a", ty.f32(), Construct(ty.f32()), {Override()});
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+
+ auto* sem_a = Sem().Get(a);
+ ASSERT_NE(sem_a, nullptr);
+ EXPECT_EQ(sem_a->Declaration(), a);
+ EXPECT_TRUE(sem_a->IsPipelineConstant());
+ EXPECT_EQ(sem_a->ConstantId(), 0u);
+}
+
+TEST_F(ResolverPipelineOverridableConstantTest, WithAndWithoutIds) {
+ std::vector<ast::Variable*> variables;
+ variables.push_back(
+ GlobalConst("a", ty.f32(), Construct(ty.f32()), {Override()}));
+ variables.push_back(
+ GlobalConst("b", ty.f32(), Construct(ty.f32()), {Override()}));
+ variables.push_back(
+ GlobalConst("c", ty.f32(), Construct(ty.f32()), {Override(2u)}));
+ variables.push_back(
+ GlobalConst("d", ty.f32(), Construct(ty.f32()), {Override(4u)}));
+ variables.push_back(
+ GlobalConst("e", ty.f32(), Construct(ty.f32()), {Override()}));
+ variables.push_back(
+ GlobalConst("f", ty.f32(), Construct(ty.f32()), {Override(1u)}));
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+
+ std::vector<uint16_t> constant_ids;
+ for (auto* var : variables) {
+ auto* sem = Sem().Get(var);
+ ASSERT_NE(sem, nullptr);
+ constant_ids.push_back(sem->ConstantId());
+ }
+ EXPECT_THAT(constant_ids, UnorderedElementsAre(0u, 3u, 2u, 4u, 5u, 1u));
+}
+
+TEST_F(ResolverPipelineOverridableConstantTest, DuplicateIds) {
+ GlobalConst("a", ty.f32(), Construct(ty.f32()),
+ {Override(Source{{12, 34}}, 7u)});
+ GlobalConst("b", ty.f32(), Construct(ty.f32()),
+ {Override(Source{{56, 78}}, 7u)});
+
+ EXPECT_FALSE(r()->Resolve());
+
+ EXPECT_EQ(r()->error(), R"(56:78 error: pipeline constant IDs must be unique
+12:34 note: a pipeline constant with an ID of 7 was previously declared here:)");
+}
+
+TEST_F(ResolverPipelineOverridableConstantTest, IdTooLarge) {
+ GlobalConst("a", ty.f32(), Construct(ty.f32()),
+ {Override(Source{{12, 34}}, 65536u)});
+
+ EXPECT_FALSE(r()->Resolve());
+
+ EXPECT_EQ(r()->error(),
+ "12:34 error: pipeline constant IDs must be between 0 and 65535");
+}
+
+} // namespace
+} // namespace resolver
+} // namespace tint
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index b38c2d1..328cb15 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -519,6 +519,13 @@
for (auto* deco : var->decorations()) {
Mark(deco);
+
+ if (auto* override_deco = deco->As<ast::OverrideDecoration>()) {
+ // Track the constant IDs that are specified in the shader.
+ if (override_deco->HasValue()) {
+ constant_ids_.emplace(override_deco->value(), info);
+ }
+ }
}
if (auto bp = var->binding_point()) {
@@ -543,7 +550,30 @@
bool Resolver::ValidateGlobalVariable(const VariableInfo* info) {
for (auto* deco : info->declaration->decorations()) {
if (info->declaration->is_const()) {
- if (!deco->Is<ast::OverrideDecoration>()) {
+ if (auto* override_deco = deco->As<ast::OverrideDecoration>()) {
+ if (override_deco->HasValue()) {
+ uint32_t id = override_deco->value();
+ auto itr = constant_ids_.find(id);
+ if (itr != constant_ids_.end() && itr->second != info) {
+ diagnostics_.add_error("pipeline constant IDs must be unique",
+ deco->source());
+ diagnostics_.add_note("a pipeline constant with an ID of " +
+ std::to_string(id) +
+ " was previously declared "
+ "here:",
+ ast::GetDecoration<ast::OverrideDecoration>(
+ itr->second->declaration->decorations())
+ ->source());
+ return false;
+ }
+ if (id > 65535) {
+ diagnostics_.add_error(
+ "pipeline constant IDs must be between 0 and 65535",
+ deco->source());
+ return false;
+ }
+ }
+ } else {
diagnostics_.add_error("decoration is not valid for constants",
deco->source());
return false;
@@ -2244,12 +2274,42 @@
}
}
+ // The next pipeline constant ID to try to allocate.
+ uint16_t next_constant_id = 0;
+
// Create semantic nodes for all ast::Variables
for (auto it : variable_to_info_) {
auto* var = it.first;
auto* info = it.second;
- auto* sem_var =
- builder_->create<sem::Variable>(var, info->type, info->storage_class);
+
+ sem::Variable* sem_var = nullptr;
+
+ if (auto* override_deco =
+ ast::GetDecoration<ast::OverrideDecoration>(var->decorations())) {
+ // Create a pipeline overridable constant.
+ uint16_t constant_id;
+ if (override_deco->HasValue()) {
+ constant_id = override_deco->value();
+ } else {
+ // No ID was specified, so allocate the next available ID.
+ constant_id = next_constant_id;
+ while (constant_ids_.count(constant_id)) {
+ if (constant_id == UINT16_MAX) {
+ TINT_ICE(builder_->Diagnostics())
+ << "no more pipeline constant IDs available";
+ return;
+ }
+ constant_id++;
+ }
+ next_constant_id = constant_id + 1;
+ }
+
+ sem_var = builder_->create<sem::Variable>(var, info->type, constant_id);
+ } else {
+ sem_var =
+ builder_->create<sem::Variable>(var, info->type, info->storage_class);
+ }
+
std::vector<const sem::VariableUser*> users;
for (auto* user : info->users) {
// Create semantic node for the identifier expression if necessary
diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h
index 95a7365..1f367f1 100644
--- a/src/resolver/resolver.h
+++ b/src/resolver/resolver.h
@@ -347,6 +347,7 @@
std::unordered_map<const ast::Expression*, ExpressionInfo> expr_info_;
std::unordered_map<Symbol, sem::Type*> named_types_;
std::unordered_set<const ast::Node*> marked_;
+ std::unordered_map<uint32_t, const VariableInfo*> constant_ids_;
FunctionInfo* current_function_ = nullptr;
sem::Statement* current_statement_ = nullptr;
BlockAllocator<VariableInfo> variable_infos_;
diff --git a/src/sem/variable.cc b/src/sem/variable.cc
index 363785e..83210cc 100644
--- a/src/sem/variable.cc
+++ b/src/sem/variable.cc
@@ -26,7 +26,19 @@
Variable::Variable(const ast::Variable* declaration,
const sem::Type* type,
ast::StorageClass storage_class)
- : declaration_(declaration), type_(type), storage_class_(storage_class) {}
+ : declaration_(declaration),
+ type_(type),
+ storage_class_(storage_class),
+ is_pipeline_constant_(false) {}
+
+Variable::Variable(const ast::Variable* declaration,
+ const sem::Type* type,
+ uint16_t constant_id)
+ : declaration_(declaration),
+ type_(type),
+ storage_class_(ast::StorageClass::kNone),
+ is_pipeline_constant_(true),
+ constant_id_(constant_id) {}
Variable::~Variable() = default;
diff --git a/src/sem/variable.h b/src/sem/variable.h
index 2090ee2..ba6118f 100644
--- a/src/sem/variable.h
+++ b/src/sem/variable.h
@@ -37,7 +37,7 @@
/// Variable holds the semantic information for variables.
class Variable : public Castable<Variable, Node> {
public:
- /// Constructor
+ /// Constructor for variables and non-overridable constants
/// @param declaration the AST declaration node
/// @param type the variable type
/// @param storage_class the variable storage class
@@ -45,6 +45,14 @@
const sem::Type* type,
ast::StorageClass storage_class);
+ /// Constructor for overridable pipeline constants
+ /// @param declaration the AST declaration node
+ /// @param type the variable type
+ /// @param constant_id the pipeline constant ID
+ Variable(const ast::Variable* declaration,
+ const sem::Type* type,
+ uint16_t constant_id);
+
/// Destructor
~Variable() override;
@@ -63,11 +71,19 @@
/// @param user the user to add
void AddUser(const VariableUser* user) { users_.emplace_back(user); }
+ /// @returns true if this variable is an overridable pipeline constant
+ bool IsPipelineConstant() const { return is_pipeline_constant_; }
+
+ /// @returns the pipeline constant ID associated with the variable
+ uint32_t ConstantId() const { return constant_id_; }
+
private:
const ast::Variable* const declaration_;
const sem::Type* const type_;
ast::StorageClass const storage_class_;
std::vector<const VariableUser*> users_;
+ const bool is_pipeline_constant_;
+ const uint16_t constant_id_ = 0;
};
/// VariableUser holds the semantic information for an identifier expression
diff --git a/test/BUILD.gn b/test/BUILD.gn
index 4f6bf11..f5c3440 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -257,6 +257,7 @@
"../src/resolver/intrinsic_test.cc",
"../src/resolver/is_host_shareable_test.cc",
"../src/resolver/is_storeable_test.cc",
+ "../src/resolver/pipeline_overridable_constant_test.cc",
"../src/resolver/resolver_test.cc",
"../src/resolver/resolver_test_helper.cc",
"../src/resolver/resolver_test_helper.h",
