Skip to content

Commit

Permalink
Rework Resolver so that we construct semantic types in a single pass.
Browse files Browse the repository at this point in the history
The semantic nodes cannot be fully immutable, as they contain cyclic
references. Remove Resolver::CreateSemanticNodes(), and instead
construct and mutate the semantic nodes in the single traversal pass.

Give up on trying to maintain the 'authored' type names (aliased names).
These are a nightmare to maintain, and provided limited use.

Significantly simplfies the Resolver, and allows us to generate more
semantic to semantic references, reducing sem -> ast -> sem hops.

Note: This change introduces constant value propagation across constant
variables. This is unlocked by the earlier construction of the
sem::Variable.

Change-Id: I592092fdc47fe24d30e512952511c9ab7c16d7a1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68406
Kokoro: Kokoro <[email protected]>
Commit-Queue: Ben Clayton <[email protected]>
Reviewed-by: Antonio Maiorano <[email protected]>
  • Loading branch information
ben-clayton authored and Tint LUCI CQ committed Nov 5, 2021
1 parent 2423df3 commit a9156ff
Show file tree
Hide file tree
Showing 43 changed files with 1,424 additions and 1,526 deletions.
10 changes: 5 additions & 5 deletions fuzzers/tint_ast_fuzzer/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "src/castable.h"
#include "src/program.h"
#include "src/sem/block_statement.h"
#include "src/sem/function.h"
#include "src/sem/statement.h"
#include "src/sem/variable.h"

Expand Down Expand Up @@ -74,16 +75,15 @@ std::vector<const sem::Variable*> GetAllVarsInScope(
}

// Process function parameters.
for (const auto* param : curr_stmt->Function()->params) {
const auto* sem_var = program.Sem().Get(param);
if (pred(sem_var)) {
result.push_back(sem_var);
for (const auto* param : curr_stmt->Function()->Parameters()) {
if (pred(param)) {
result.push_back(param);
}
}

// Global variables do not belong to any ast::BlockStatement.
for (const auto* global_decl : program.AST().GlobalDeclarations()) {
if (global_decl == curr_stmt->Function()) {
if (global_decl == curr_stmt->Function()->Declaration()) {
// The same situation as in the previous loop. The current function has
// been reached. If there are any variables declared below, they won't be
// visible in this function. Thus, exit the loop.
Expand Down
17 changes: 9 additions & 8 deletions src/inspector/inspector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -827,11 +827,11 @@ void Inspector::GenerateSamplerTargets() {
}

auto* call_func = call->Stmt()->Function();
std::vector<Symbol> entry_points;
if (call_func->IsEntryPoint()) {
entry_points = {call_func->symbol};
std::vector<const sem::Function*> entry_points;
if (call_func->Declaration()->IsEntryPoint()) {
entry_points = {call_func};
} else {
entry_points = sem.Get(call_func)->AncestorEntryPoints();
entry_points = call_func->AncestorEntryPoints();
}

if (entry_points.empty()) {
Expand All @@ -854,8 +854,9 @@ void Inspector::GenerateSamplerTargets() {
sampler->Declaration()->BindingPoint().group->value,
sampler->Declaration()->BindingPoint().binding->value};

for (auto entry_point : entry_points) {
const auto& ep_name = program_->Symbols().NameFor(entry_point);
for (auto* entry_point : entry_points) {
const auto& ep_name =
program_->Symbols().NameFor(entry_point->Declaration()->symbol);
(*sampler_targets_)[ep_name].add(
{sampler_binding_point, texture_binding_point});
}
Expand Down Expand Up @@ -911,8 +912,8 @@ void Inspector::GetOriginatingResources(
// is not called. Ignore.
return;
}
for (auto* call_expr : func->CallSites()) {
callsites.add(call_expr);
for (auto* call : func->CallSites()) {
callsites.add(call->Declaration());
}
// Need to evaluate each function call with the group of
// expressions, so move on to the next expression.
Expand Down
4 changes: 2 additions & 2 deletions src/intrinsic_table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1070,8 +1070,8 @@ const sem::Intrinsic* Impl::Match(sem::IntrinsicType intrinsic_type,
ast::StorageClass::kNone, ast::Access::kUndefined, p.usage));
}
return builder.create<sem::Intrinsic>(
intrinsic.type, intrinsic.return_type,
std::move(params), intrinsic.supported_stages, intrinsic.is_deprecated);
intrinsic.type, intrinsic.return_type, std::move(params),
intrinsic.supported_stages, intrinsic.is_deprecated);
});
}

Expand Down
5 changes: 3 additions & 2 deletions src/resolver/array_accessor_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,9 @@ TEST_F(ResolverArrayAccessorTest, EXpr_Deref_FuncBadParent) {
Func("func", {p}, ty.f32(), {Decl(idx), Decl(x), Return(x)});

EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: cannot index type 'ptr<function, vec4<f32>>'");
EXPECT_EQ(
r()->error(),
"12:34 error: cannot index type 'ptr<function, vec4<f32>, read_write>'");
}

TEST_F(ResolverArrayAccessorTest, Exr_Deref_BadParent) {
Expand Down
4 changes: 2 additions & 2 deletions src/resolver/assignment_validation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ TEST_F(ResolverAssignmentValidationTest,
ASSERT_FALSE(r()->Resolve());

EXPECT_EQ(r()->error(),
"12:34 error: cannot assign 'array<f32, len>' to 'array<f32, 4>'");
"12:34 error: cannot assign 'array<f32, 5>' to 'array<f32, 4>'");
}

TEST_F(ResolverAssignmentValidationTest,
Expand Down Expand Up @@ -332,7 +332,7 @@ TEST_F(ResolverAssignmentValidationTest, AssignToPhony_DynamicArray_Fail) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(
r()->error(),
"12:34 error: cannot assign 'ref<storage, array<i32>, read>' to '_'. "
"12:34 error: cannot assign 'array<i32>' to '_'. "
"'_' can only be assigned a constructible, pointer, texture or sampler "
"type");
}
Expand Down
16 changes: 5 additions & 11 deletions src/resolver/compound_statement_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ TEST_F(ResolverCompoundStatementTest, FunctionBlock) {
ASSERT_TRUE(s->Block()->Is<sem::FunctionBlockStatement>());
EXPECT_EQ(s->Block(), s->FindFirstParent<sem::BlockStatement>());
EXPECT_EQ(s->Block(), s->FindFirstParent<sem::FunctionBlockStatement>());
EXPECT_EQ(s->Block()->As<sem::FunctionBlockStatement>()->Function(), f);
EXPECT_EQ(s->Function()->Declaration(), f);
EXPECT_EQ(s->Block()->Parent(), nullptr);
}

Expand Down Expand Up @@ -74,8 +74,7 @@ TEST_F(ResolverCompoundStatementTest, Block) {
EXPECT_EQ(s->Block()->Parent(),
s->FindFirstParent<sem::FunctionBlockStatement>());
ASSERT_TRUE(s->Block()->Parent()->Is<sem::FunctionBlockStatement>());
EXPECT_EQ(
s->Block()->Parent()->As<sem::FunctionBlockStatement>()->Function(), f);
EXPECT_EQ(s->Function()->Declaration(), f);
EXPECT_EQ(s->Block()->Parent()->Parent(), nullptr);
}
}
Expand Down Expand Up @@ -118,7 +117,7 @@ TEST_F(ResolverCompoundStatementTest, Loop) {
EXPECT_TRUE(
Is<sem::FunctionBlockStatement>(s->Parent()->Parent()->Parent()));

EXPECT_EQ(s->FindFirstParent<sem::FunctionBlockStatement>()->Function(), f);
EXPECT_EQ(s->Function()->Declaration(), f);

EXPECT_EQ(s->Parent()->Parent()->Parent()->Parent(), nullptr);
}
Expand All @@ -144,7 +143,7 @@ TEST_F(ResolverCompoundStatementTest, Loop) {
s->FindFirstParent<sem::FunctionBlockStatement>());
EXPECT_TRUE(Is<sem::FunctionBlockStatement>(
s->Parent()->Parent()->Parent()->Parent()));
EXPECT_EQ(s->FindFirstParent<sem::FunctionBlockStatement>()->Function(), f);
EXPECT_EQ(s->Function()->Declaration(), f);

EXPECT_EQ(s->Parent()->Parent()->Parent()->Parent()->Parent(), nullptr);
}
Expand Down Expand Up @@ -213,12 +212,7 @@ TEST_F(ResolverCompoundStatementTest, ForLoop) {
Is<sem::FunctionBlockStatement>(s->Block()->Parent()->Parent()));
EXPECT_EQ(s->Block()->Parent()->Parent(),
s->FindFirstParent<sem::FunctionBlockStatement>());
EXPECT_EQ(s->Block()
->Parent()
->Parent()
->As<sem::FunctionBlockStatement>()
->Function(),
f);
EXPECT_EQ(s->Function()->Declaration(), f);
EXPECT_EQ(s->Block()->Parent()->Parent()->Parent(), nullptr);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/resolver/function_validation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ TEST_F(ResolverFunctionValidationTest,
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: return statement type must match its function return "
"type, returned 'u32', expected 'myf32'");
"type, returned 'u32', expected 'f32'");
}

TEST_F(ResolverFunctionValidationTest, CannotCallEntryPoint) {
Expand Down
15 changes: 10 additions & 5 deletions src/resolver/ptr_ref_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,16 @@ TEST_F(ResolverPtrRefTest, DefaultPtrStorageClass) {

EXPECT_TRUE(r()->Resolve()) << r()->error();

ASSERT_TRUE(TypeOf(function_ptr)->Is<sem::Pointer>());
ASSERT_TRUE(TypeOf(private_ptr)->Is<sem::Pointer>());
ASSERT_TRUE(TypeOf(workgroup_ptr)->Is<sem::Pointer>());
ASSERT_TRUE(TypeOf(uniform_ptr)->Is<sem::Pointer>());
ASSERT_TRUE(TypeOf(storage_ptr)->Is<sem::Pointer>());
ASSERT_TRUE(TypeOf(function_ptr)->Is<sem::Pointer>())
<< "function_ptr is " << TypeOf(function_ptr)->TypeInfo().name;
ASSERT_TRUE(TypeOf(private_ptr)->Is<sem::Pointer>())
<< "private_ptr is " << TypeOf(private_ptr)->TypeInfo().name;
ASSERT_TRUE(TypeOf(workgroup_ptr)->Is<sem::Pointer>())
<< "workgroup_ptr is " << TypeOf(workgroup_ptr)->TypeInfo().name;
ASSERT_TRUE(TypeOf(uniform_ptr)->Is<sem::Pointer>())
<< "uniform_ptr is " << TypeOf(uniform_ptr)->TypeInfo().name;
ASSERT_TRUE(TypeOf(storage_ptr)->Is<sem::Pointer>())
<< "storage_ptr is " << TypeOf(storage_ptr)->TypeInfo().name;

EXPECT_EQ(TypeOf(function_ptr)->As<sem::Pointer>()->Access(),
ast::Access::kReadWrite);
Expand Down
2 changes: 1 addition & 1 deletion src/resolver/ptr_ref_validation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ TEST_F(ResolverPtrRefValidationTest, InferredPtrAccessMismatch) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: cannot initialize let of type "
"'ptr<storage, i32>' with value of type "
"'ptr<storage, i32, read>' with value of type "
"'ptr<storage, i32, read_write>'");
}

Expand Down
Loading

0 comments on commit a9156ff

Please sign in to comment.