Implement Pointers and References This change implements pointers and references as described by the WGSL specification change in https://github.com/gpuweb/gpuweb/pull/1569. reader/spirv: * Now emits address-of `&expr` and indirection `*expr` operators as needed. * As an identifier may now resolve to a pointer or reference type depending on whether the declaration is a `var`, `let` or parameter, `Function::identifier_values_` has been changed from an ID set to an ID -> Type* map. resolver: * Now correctly resolves all expressions to either a value type, reference type or pointer type. * Validates pointer / reference rules on assignment, `var` and `let` construction, and usage. * Handles the address-of and indirection operators. * No longer does any implicit loads of pointer types. * Storage class validation is still TODO (crbug.com/tint/809) writer/spirv: * Correctly handles variables and expressions of pointer and reference types, emitting OpLoads where necessary. test: * Lots of new test cases Fixed: tint:727 Change-Id: I77d3281590e35e5a3122f5b74cdeb71a6fe51f74 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50740 Commit-Queue: Ben Clayton <bclayton@chromium.org> Kokoro: Kokoro <noreply+kokoro@google.com> Reviewed-by: David Neto <dneto@google.com>
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 77d3eeb..9927bdd 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt
@@ -550,6 +550,8 @@ resolver/intrinsic_test.cc resolver/is_host_shareable_test.cc resolver/is_storeable_test.cc + resolver/ptr_ref_test.cc + resolver/ptr_ref_validation_test.cc resolver/pipeline_overridable_constant_test.cc resolver/resolver_test_helper.cc resolver/resolver_test_helper.h @@ -561,6 +563,8 @@ resolver/type_constructor_validation_test.cc resolver/type_validation_test.cc resolver/validation_test.cc + resolver/var_let_test.cc + resolver/var_let_validation_test.cc scope_stack_test.cc sem/intrinsic_test.cc symbol_table_test.cc
diff --git a/src/inspector/inspector.cc b/src/inspector/inspector.cc index eb79153..b8ac9b4 100644 --- a/src/inspector/inspector.cc +++ b/src/inspector/inspector.cc
@@ -227,7 +227,7 @@ stage_variable.name = name; stage_variable.component_type = ComponentType::kUnknown; - auto* type = var->Type()->UnwrapAll(); + auto* type = var->Type()->UnwrapRef(); if (type->is_float_scalar_or_vector() || type->is_float_matrix()) { stage_variable.component_type = ComponentType::kFloat; } else if (type->is_unsigned_scalar_or_vector()) { @@ -400,7 +400,7 @@ auto* var = ruv.first; auto binding_info = ruv.second; - auto* unwrapped_type = var->Type()->UnwrapAccess(); + auto* unwrapped_type = var->Type()->UnwrapRef(); auto* str = unwrapped_type->As<sem::Struct>(); if (str == nullptr) { continue; @@ -522,7 +522,7 @@ entry.bind_group = binding_info.group->value(); entry.binding = binding_info.binding->value(); - auto* texture_type = var->Type()->UnwrapAccess()->As<sem::Texture>(); + auto* texture_type = var->Type()->UnwrapRef()->As<sem::Texture>(); entry.dim = TypeTextureDimensionToResourceBindingTextureDimension( texture_type->dim()); @@ -550,7 +550,7 @@ entry.bind_group = binding_info.group->value(); entry.binding = binding_info.binding->value(); - auto* texture_type = var->Type()->UnwrapAccess()->As<sem::Texture>(); + auto* texture_type = var->Type()->UnwrapRef()->As<sem::Texture>(); entry.dim = TypeTextureDimensionToResourceBindingTextureDimension( texture_type->dim()); @@ -584,7 +584,7 @@ return; } - auto* unwrapped_type = type->UnwrapAll(); + auto* unwrapped_type = type->UnwrapRef(); if (auto* struct_ty = unwrapped_type->As<sem::Struct>()) { // Recurse into members. @@ -641,7 +641,7 @@ continue; } - auto* str = var->Type()->UnwrapAccess()->As<sem::Struct>(); + auto* str = var->Type()->UnwrapRef()->As<sem::Struct>(); if (!str) { continue; } @@ -685,7 +685,7 @@ entry.bind_group = binding_info.group->value(); entry.binding = binding_info.binding->value(); - auto* texture_type = var->Type()->UnwrapAccess()->As<sem::Texture>(); + auto* texture_type = var->Type()->UnwrapRef()->As<sem::Texture>(); entry.dim = TypeTextureDimensionToResourceBindingTextureDimension( texture_type->dim()); @@ -717,7 +717,7 @@ auto* var = ref.first; auto binding_info = ref.second; - auto* texture_type = var->Type()->As<sem::StorageTexture>(); + auto* texture_type = var->Type()->UnwrapRef()->As<sem::StorageTexture>(); if (read_only != (texture_type->access_control() == ast::AccessControl::kReadOnly)) {
diff --git a/src/intrinsic_table.cc b/src/intrinsic_table.cc index 802034f..e313d0a 100644 --- a/src/intrinsic_table.cc +++ b/src/intrinsic_table.cc
@@ -1305,7 +1305,7 @@ ss << ", "; } first = false; - ss << arg->FriendlyName(builder.Symbols()); + ss << arg->UnwrapRef()->FriendlyName(builder.Symbols()); } } ss << ")"; @@ -1391,14 +1391,7 @@ return nullptr; } - auto* arg_ty = args[i]; - if (auto* ptr = arg_ty->As<sem::Pointer>()) { - if (!parameters[i].matcher->ExpectsPointer()) { - // Argument is a pointer, but the matcher isn't expecting one. - // Perform an implicit dereference. - arg_ty = ptr->StoreType(); - } - } + auto* arg_ty = args[i]->UnwrapRef(); if (parameters[i].matcher->Match(matcher_state, arg_ty)) { // A correct parameter match is scored higher than number of parameters to // arguments.
diff --git a/src/intrinsic_table_test.cc b/src/intrinsic_table_test.cc index 5bd2f5d..8065f03 100644 --- a/src/intrinsic_table_test.cc +++ b/src/intrinsic_table_test.cc
@@ -19,6 +19,7 @@ #include "src/sem/depth_texture_type.h" #include "src/sem/external_texture_type.h" #include "src/sem/multisampled_texture_type.h" +#include "src/sem/reference_type.h" #include "src/sem/sampled_texture_type.h" #include "src/sem/storage_texture_type.h" @@ -345,10 +346,11 @@ ASSERT_THAT(result.diagnostics.str(), HasSubstr("no matching call")); } -TEST_F(IntrinsicTableTest, MatchAutoPointerDereference) { - auto result = - table->Lookup(*this, IntrinsicType::kCos, - {ty.pointer<f32>(ast::StorageClass::kNone)}, Source{}); +TEST_F(IntrinsicTableTest, ImplicitLoadOnReference) { + auto result = table->Lookup( + *this, IntrinsicType::kCos, + {create<sem::Reference>(create<sem::F32>(), ast::StorageClass::kNone)}, + Source{}); ASSERT_NE(result.intrinsic, nullptr); ASSERT_EQ(result.diagnostics.str(), ""); EXPECT_THAT(result.intrinsic->Type(), IntrinsicType::kCos);
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index 805fc56..c3d6b06 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc
@@ -1088,15 +1088,15 @@ ast::VariableList ast_params; function_.ForEachParam( [this, &ast_params](const spvtools::opt::Instruction* param) { - auto* ast_type = parser_impl_.ConvertType(param->type_id()); - if (ast_type != nullptr) { + auto* type = parser_impl_.ConvertType(param->type_id()); + if (type != nullptr) { auto* ast_param = parser_impl_.MakeVariable( - param->result_id(), ast::StorageClass::kNone, ast_type, true, - nullptr, ast::DecorationList{}); + param->result_id(), ast::StorageClass::kNone, type, true, nullptr, + ast::DecorationList{}); // Parameters are treated as const declarations. ast_params.emplace_back(ast_param); // The value is accessible by name. - identifier_values_.insert(param->result_id()); + identifier_types_.emplace(param->result_id(), type); } else { // We've already logged an error and emitted a diagnostic. Do nothing // here. @@ -2194,8 +2194,8 @@ constructor, ast::DecorationList{}); auto* var_decl_stmt = create<ast::VariableDeclStatement>(Source{}, var); AddStatement(var_decl_stmt); - // Save this as an already-named value. - identifier_values_.insert(inst.result_id()); + auto* var_type = ty_.Reference(var_store_type, ast::StorageClass::kNone); + identifier_types_.emplace(inst.result_id(), var_type); } return success(); } @@ -2246,7 +2246,15 @@ create<ast::IdentifierExpression>( Source{}, builder_.Symbols().Register(name))}; } - if (identifier_values_.count(id) || parser_impl_.IsScalarSpecConstant(id)) { + auto type_it = identifier_types_.find(id); + if (type_it != identifier_types_.end()) { + auto name = namer_.Name(id); + auto* type = type_it->second; + return TypedExpression{type, + create<ast::IdentifierExpression>( + Source{}, builder_.Symbols().Register(name))}; + } + if (parser_impl_.IsScalarSpecConstant(id)) { auto name = namer_.Name(id); return TypedExpression{ parser_impl_.ConvertType(def_use_mgr_->GetDef(id)->type_id()), @@ -2271,9 +2279,10 @@ case SpvOpVariable: { // This occurs for module-scope variables. auto name = namer_.Name(inst->result_id()); - return TypedExpression{parser_impl_.ConvertType(inst->type_id()), - create<ast::IdentifierExpression>( - Source{}, builder_.Symbols().Register(name))}; + return TypedExpression{ + parser_impl_.ConvertType(inst->type_id(), PtrAs::Ref), + create<ast::IdentifierExpression>(Source{}, + builder_.Symbols().Register(name))}; } case SpvOpUndef: // Substitute a null value for undef. @@ -2624,7 +2633,7 @@ // just like in the original SPIR-V. PushTrueGuard(construct->end_id); } else { - // Add a flow guard around the blocks in the premege area. + // Add a flow guard around the blocks in the premerge area. PushGuard(guard_name, construct->end_id); } } @@ -2836,7 +2845,7 @@ const auto true_dest = terminator.GetSingleWordInOperand(1); const auto false_dest = terminator.GetSingleWordInOperand(2); if (true_dest == false_dest) { - // This is like an uncondtional branch. + // This is like an unconditional branch. AddStatement(MakeBranch(block_info, *GetBlockInfo(true_dest))); return true; } @@ -3064,14 +3073,14 @@ for (auto id : sorted_by_index(block_info.hoisted_ids)) { const auto* def_inst = def_use_mgr_->GetDef(id); TINT_ASSERT(def_inst); - auto* ast_type = + auto* storage_type = RemapStorageClass(parser_impl_.ConvertType(def_inst->type_id()), id); AddStatement(create<ast::VariableDeclStatement>( Source{}, - parser_impl_.MakeVariable(id, ast::StorageClass::kNone, ast_type, false, - nullptr, ast::DecorationList{}))); - // Save this as an already-named value. - identifier_values_.insert(id); + parser_impl_.MakeVariable(id, ast::StorageClass::kNone, storage_type, + false, nullptr, ast::DecorationList{}))); + auto* type = ty_.Reference(storage_type, ast::StorageClass::kNone); + identifier_types_.emplace(id, type); } // Emit declarations of phi state variables, in index order. for (auto id : sorted_by_index(block_info.phis_needing_state_vars)) { @@ -3131,25 +3140,29 @@ bool FunctionEmitter::EmitConstDefinition( const spvtools::opt::Instruction& inst, - TypedExpression ast_expr) { - if (!ast_expr) { + TypedExpression expr) { + if (!expr) { return false; } + if (expr.type->Is<Reference>()) { + // `let` declarations cannot hold references, so we need to take the address + // of the RHS, and make the `let` be a pointer. + expr = AddressOf(expr); + } auto* ast_const = parser_impl_.MakeVariable( - inst.result_id(), ast::StorageClass::kNone, ast_expr.type, true, - ast_expr.expr, ast::DecorationList{}); + inst.result_id(), ast::StorageClass::kNone, expr.type, true, expr.expr, + ast::DecorationList{}); if (!ast_const) { return false; } AddStatement(create<ast::VariableDeclStatement>(Source{}, ast_const)); - // Save this as an already-named value. - identifier_values_.insert(inst.result_id()); + identifier_types_.emplace(inst.result_id(), expr.type); return success(); } bool FunctionEmitter::EmitConstDefOrWriteToHoistedVar( const spvtools::opt::Instruction& inst, - TypedExpression ast_expr) { + TypedExpression expr) { const auto result_id = inst.result_id(); const auto* def_info = GetDefInfo(result_id); if (def_info && def_info->requires_hoisted_def) { @@ -3159,10 +3172,10 @@ Source{}, create<ast::IdentifierExpression>(Source{}, builder_.Symbols().Register(name)), - ast_expr.expr)); + expr.expr)); return true; } - return EmitConstDefinition(inst, ast_expr); + return EmitConstDefinition(inst, expr); } bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { @@ -3283,6 +3296,12 @@ return false; } + if (lhs.type->Is<Pointer>()) { + // LHS of an assignment must be a reference type. + // Convert the LHS to a reference by dereferencing it. + lhs = Dereference(lhs); + } + AddStatement( create<ast::AssignmentStatement>(Source{}, lhs.expr, rhs.expr)); return success(); @@ -3343,16 +3362,21 @@ return false; } - // The load result type is the pointee type of its operand. - TINT_ASSERT(expr.type->Is<Pointer>()); - expr.type = expr.type->As<Pointer>()->type; + // The load result type is the storage type of its operand. + if (expr.type->Is<Pointer>()) { + expr = Dereference(expr); + } else if (auto* ref = expr.type->As<Reference>()) { + expr.type = ref->type; + } else { + Fail() << "OpLoad expression is not a pointer or reference"; + return false; + } + return EmitConstDefOrWriteToHoistedVar(inst, expr); } case SpvOpCopyMemory: { // Generate an assignment. - // TODO(dneto): When supporting ptr-ref, the LHS pointer and RHS pointer - // map to reference types in WGSL. auto lhs = MakeOperand(inst, 0); auto rhs = MakeOperand(inst, 1); // Ignore any potential memory operands. Currently they are all for @@ -3367,6 +3391,15 @@ if (!success()) { return false; } + + // LHS and RHS pointers must be reference types in WGSL. + if (lhs.type->Is<Pointer>()) { + lhs = Dereference(lhs); + } + if (rhs.type->Is<Pointer>()) { + rhs = Dereference(rhs); + } + AddStatement( create<ast::AssignmentStatement>(Source{}, lhs.expr, rhs.expr)); return success(); @@ -3386,6 +3419,11 @@ if (!expr) { return false; } + if (expr.type->Is<Reference>()) { + // If the source is a reference, then we need to take the address of the + // expression. + expr = AddressOf(expr); + } expr.type = RemapStorageClass(expr.type, result_id); return EmitConstDefOrWriteToHoistedVar(inst, expr); } @@ -3782,7 +3820,7 @@ auto name = namer_.Name(base_id); current_expr.expr = create<ast::IdentifierExpression>( Source{}, builder_.Symbols().Register(name)); - current_expr.type = parser_impl_.ConvertType(ptr_ty_id); + current_expr.type = parser_impl_.ConvertType(ptr_ty_id, PtrAs::Ref); } } @@ -3898,10 +3936,9 @@ } const auto pointer_type_id = type_mgr_->FindPointerToType(pointee_type_id, storage_class); - auto* ast_pointer_type = parser_impl_.ConvertType(pointer_type_id); - TINT_ASSERT(ast_pointer_type); - TINT_ASSERT(ast_pointer_type->Is<Pointer>()); - current_expr = TypedExpression{ast_pointer_type, next_expr}; + auto* type = parser_impl_.ConvertType(pointer_type_id, PtrAs::Ref); + TINT_ASSERT(type && type->Is<Reference>()); + current_expr = TypedExpression{type, next_expr}; } return current_expr; } @@ -3932,7 +3969,7 @@ // A SPIR-V composite extract is a single instruction with multiple // literal indices walking down into composites. // A SPIR-V composite insert is similar but also tells you what component - // to inject. This function is respnosible for the the walking-into part + // to inject. This function is responsible for the the walking-into part // of composite-insert. // // The Tint AST represents this as ever-deeper nested indexing expressions. @@ -4476,15 +4513,24 @@ auto* function = create<ast::IdentifierExpression>( Source{}, builder_.Symbols().Register(name)); - ast::ExpressionList params; + ast::ExpressionList args; for (uint32_t iarg = 1; iarg < inst.NumInOperands(); ++iarg) { - params.emplace_back(MakeOperand(inst, iarg).expr); + auto expr = MakeOperand(inst, iarg); + if (!expr) { + return false; + } + if (expr.type->Is<Reference>()) { + // Functions cannot use references as parameters, so we need to pass by + // pointer. + expr = AddressOf(expr); + } + args.emplace_back(expr.expr); } if (failed()) { return false; } auto* call_expr = - create<ast::CallExpression>(Source{}, function, std::move(params)); + create<ast::CallExpression>(Source{}, function, std::move(args)); auto* result_type = parser_impl_.ConvertType(inst.type_id()); if (!result_type) { return Fail() << "internal error: no mapped type result of call: " @@ -5394,6 +5440,32 @@ {ast_type, create<ast::IdentifierExpression>(registered_temp_name)}); } +TypedExpression FunctionEmitter::AddressOf(TypedExpression expr) { + auto* ref = expr.type->As<Reference>(); + if (!ref) { + Fail() << "AddressOf() called on non-reference type"; + return {}; + } + return { + ty_.Pointer(ref->type, ref->storage_class), + create<ast::UnaryOpExpression>(Source{}, ast::UnaryOp::kAddressOf, + expr.expr), + }; +} + +TypedExpression FunctionEmitter::Dereference(TypedExpression expr) { + auto* ptr = expr.type->As<Pointer>(); + if (!ptr) { + Fail() << "Dereference() called on non-pointer type"; + return {}; + } + return { + ptr->type, + create<ast::UnaryOpExpression>(Source{}, ast::UnaryOp::kIndirection, + expr.expr), + }; +} + FunctionEmitter::FunctionDeclaration::FunctionDeclaration() = default; FunctionEmitter::FunctionDeclaration::~FunctionDeclaration() = default;
diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h index edff907..940e84a 100644 --- a/src/reader/spirv/function.h +++ b/src/reader/spirv/function.h
@@ -1127,6 +1127,16 @@ /// @returns a boolean false expression. ast::Expression* MakeFalse(const Source&) const; + /// @param expr the expression to take the address of + /// @returns a TypedExpression that is the address-of `expr` (`&expr`) + /// @note `expr` must be a reference type + TypedExpression AddressOf(TypedExpression expr); + + /// @param expr the expression to dereference + /// @returns a TypedExpression that is the dereference-of `expr` (`*expr`) + /// @note `expr` must be a pointer type + TypedExpression Dereference(TypedExpression expr); + /// Creates a new `ast::Node` owned by the ProgramBuilder. /// @param args the arguments to pass to the type constructor /// @returns the node pointer @@ -1136,6 +1146,7 @@ } using StatementsStack = std::vector<StatementBlock>; + using PtrAs = ParserImpl::PtrAs; ParserImpl& parser_impl_; TypeManager& ty_; @@ -1160,8 +1171,9 @@ // lifetime of the EmitFunctionBodyStatements method. StatementsStack statements_stack_; - // The set of IDs that have already had an identifier name generated for it. - std::unordered_set<uint32_t> identifier_values_; + // The map of IDs that have already had an identifier name generated for it, + // to their Type. + std::unordered_map<uint32_t, const Type*> identifier_types_; // Mapping from SPIR-V ID that is used at most once, to its AST expression. std::unordered_map<uint32_t, TypedExpression> singly_used_values_;
diff --git a/src/reader/spirv/function_composite_test.cc b/src/reader/spirv/function_composite_test.cc index 2724fd5..4fd0f07 100644 --- a/src/reader/spirv/function_composite_test.cc +++ b/src/reader/spirv/function_composite_test.cc
@@ -923,7 +923,7 @@ VariableConst{ x_1 none - __type_name_S_1 + __type_name_S_2 { Identifier[not set]{x_40} } @@ -1178,7 +1178,10 @@ none __ptr_function__u32 { - Identifier[not set]{x_10} + UnaryOp[not set]{ + address-of + Identifier[not set]{x_10} + } } } }
diff --git a/src/reader/spirv/function_memory_test.cc b/src/reader/spirv/function_memory_test.cc index e6b1d3e..f44846a 100644 --- a/src/reader/spirv/function_memory_test.cc +++ b/src/reader/spirv/function_memory_test.cc
@@ -1019,18 +1019,24 @@ none __ptr_storage__u32 { - ArrayAccessor[not set]{ - MemberAccessor[not set]{ - Identifier[not set]{myvar} - Identifier[not set]{field1} + UnaryOp[not set]{ + address-of + ArrayAccessor[not set]{ + MemberAccessor[not set]{ + Identifier[not set]{myvar} + Identifier[not set]{field1} + } + ScalarConstructor[not set]{1u} } - ScalarConstructor[not set]{1u} } } } } Assignment{ - Identifier[not set]{x_2} + UnaryOp[not set]{ + indirection + Identifier[not set]{x_2} + } ScalarConstructor[not set]{0u} })")) << p->error(); } @@ -1082,12 +1088,15 @@ { Assignment{ Identifier[not set]{x_2} - ArrayAccessor[not set]{ - MemberAccessor[not set]{ - Identifier[not set]{myvar} - Identifier[not set]{field1} + UnaryOp[not set]{ + address-of + ArrayAccessor[not set]{ + MemberAccessor[not set]{ + Identifier[not set]{myvar} + Identifier[not set]{field1} + } + ScalarConstructor[not set]{1u} } - ScalarConstructor[not set]{1u} } } }
diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index 7f3ae37..986ca7a 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc
@@ -24,6 +24,7 @@ #include "src/ast/override_decoration.h" #include "src/ast/struct_block_decoration.h" #include "src/ast/type_name.h" +#include "src/ast/unary_op_expression.h" #include "src/reader/spirv/function.h" #include "src/sem/depth_texture_type.h" #include "src/sem/multisampled_texture_type.h" @@ -304,7 +305,7 @@ return tint::Program(std::move(builder_)); } -const Type* ParserImpl::ConvertType(uint32_t type_id) { +const Type* ParserImpl::ConvertType(uint32_t type_id, PtrAs ptr_as) { if (!success_) { return nullptr; } @@ -349,7 +350,7 @@ return maybe_generate_alias(ConvertType(type_id, spirv_type->AsStruct())); case spvtools::opt::analysis::Type::kPointer: return maybe_generate_alias( - ConvertType(type_id, spirv_type->AsPointer())); + ConvertType(type_id, ptr_as, spirv_type->AsPointer())); case spvtools::opt::analysis::Type::kFunction: // Tint doesn't have a Function type. // We need to convert the result type and parameter types. @@ -1041,6 +1042,7 @@ } const Type* ParserImpl::ConvertType(uint32_t type_id, + PtrAs ptr_as, const spvtools::opt::analysis::Pointer*) { const auto* inst = def_use_mgr_->GetDef(type_id); const auto pointee_type_id = inst->GetSingleWordInOperand(1); @@ -1051,7 +1053,7 @@ builtin_position_.storage_class = storage_class; return nullptr; } - auto* ast_elem_ty = ConvertType(pointee_type_id); + auto* ast_elem_ty = ConvertType(pointee_type_id, PtrAs::Ptr); if (ast_elem_ty == nullptr) { Fail() << "SPIR-V pointer type with ID " << type_id << " has invalid pointee type " << pointee_type_id; @@ -1079,8 +1081,14 @@ ast_storage_class = ast::StorageClass::kPrivate; } } - - return ty_.Pointer(ast_elem_ty, ast_storage_class); + switch (ptr_as) { + case PtrAs::Ref: + return ty_.Reference(ast_elem_ty, ast_storage_class); + case PtrAs::Ptr: + return ty_.Pointer(ast_elem_ty, ast_storage_class); + } + Fail() << "invalid value for ptr_as: " << static_cast<int>(ptr_as); + return nullptr; } bool ParserImpl::RegisterTypes() { @@ -1094,7 +1102,7 @@ } ConvertType(type_or_const.result_id()); } - // Manufacture a type for the gl_Position varible if we have to. + // Manufacture a type for the gl_Position variable if we have to. if ((builtin_position_.struct_type_id != 0) && (builtin_position_.position_member_pointer_type_id == 0)) { builtin_position_.position_member_pointer_type_id = @@ -1337,25 +1345,25 @@ ast::Variable* ParserImpl::MakeVariable(uint32_t id, ast::StorageClass sc, - const Type* type, + const Type* storage_type, bool is_const, ast::Expression* constructor, ast::DecorationList decorations) { - if (type == nullptr) { + if (storage_type == nullptr) { Fail() << "internal error: can't make ast::Variable for null type"; return nullptr; } if (sc == ast::StorageClass::kStorage) { bool read_only = false; - if (auto* tn = type->As<Named>()) { + if (auto* tn = storage_type->As<Named>()) { read_only = read_only_struct_types_.count(tn->name) > 0; } // Apply the access(read) or access(read_write) modifier. auto access = read_only ? ast::AccessControl::kReadOnly : ast::AccessControl::kReadWrite; - type = ty_.AccessControl(type, access); + storage_type = ty_.AccessControl(storage_type, access); } // Handle variables (textures and samplers) are always in the handle @@ -1367,15 +1375,20 @@ // In almost all cases, copy the decorations from SPIR-V to the variable. // But avoid doing so when converting pipeline IO to private variables. if (sc != ast::StorageClass::kPrivate) { - if (!ConvertDecorationsForVariable(id, &type, &decorations)) { + if (!ConvertDecorationsForVariable(id, &storage_type, &decorations)) { return nullptr; } } std::string name = namer_.Name(id); + + // Note: we're constructing the variable here with the *storage* type, + // regardless of whether this is a `let` or `var` declaration. + // `var` declarations will have a resolved type of ref<storage>, but at the + // AST level both `var` and `let` are declared with the same type. return create<ast::Variable>(Source{}, builder_.Symbols().Register(name), sc, - type->Build(builder_), is_const, constructor, - decorations); + storage_type->Build(builder_), is_const, + constructor, decorations); } bool ParserImpl::ConvertDecorationsForVariable(
diff --git a/src/reader/spirv/parser_impl.h b/src/reader/spirv/parser_impl.h index 9f3e336..14250fc 100644 --- a/src/reader/spirv/parser_impl.h +++ b/src/reader/spirv/parser_impl.h
@@ -153,6 +153,14 @@ return glsl_std_450_imports_; } + /// Desired handling of SPIR-V pointers by ConvertType() + enum class PtrAs { + // SPIR-V pointer is converted to a spirv::Pointer + Ptr, + // SPIR-V pointer is converted to a spirv::Reference + Ref + }; + /// Converts a SPIR-V type to a Tint type, and saves it for fast lookup. /// If the type is only used for builtins, then register that specially, /// and return null. If the type is a sampler, image, or sampled image, then @@ -161,8 +169,11 @@ /// On failure, logs an error and returns null. This should only be called /// after the internal representation of the module has been built. /// @param type_id the SPIR-V ID of a type. + /// @param ptr_as if the SPIR-V type is a pointer and ptr_as is equal to + /// PtrAs::Ref then a Reference will be returned, otherwise a Pointer will be + /// returned for a SPIR-V pointer /// @returns a Tint type, or nullptr - const Type* ConvertType(uint32_t type_id); + const Type* ConvertType(uint32_t type_id, PtrAs ptr_as = PtrAs::Ptr); /// Emits an alias type declaration for the given type, if necessary, and /// also updates the mapping of the SPIR-V type ID to the alias type. @@ -339,7 +350,7 @@ /// decorations, unless it's an ignorable builtin variable. /// @param id the SPIR-V result ID /// @param sc the storage class, which cannot be ast::StorageClass::kNone - /// @param type the type + /// @param storage_type the storage type of the variable /// @param is_const if true, the variable is const /// @param constructor the variable constructor /// @param decorations the variable decorations @@ -347,7 +358,7 @@ /// in the error case ast::Variable* MakeVariable(uint32_t id, ast::StorageClass sc, - const Type* type, + const Type* storage_type, bool is_const, ast::Expression* constructor, ast::DecorationList decorations); @@ -616,12 +627,15 @@ /// @param struct_ty the Tint type const Type* ConvertType(uint32_t type_id, const spvtools::opt::analysis::Struct* struct_ty); - /// Converts a specific SPIR-V type to a Tint type. Pointer case + /// Converts a specific SPIR-V type to a Tint type. Pointer / Reference case /// The pointer to gl_PerVertex maps to nullptr, and instead is recorded /// in member #builtin_position_. /// @param type_id the SPIR-V ID for the type. + /// @param ptr_as if PtrAs::Ref then a Reference will be returned, otherwise + /// Pointer /// @param ptr_ty the Tint type const Type* ConvertType(uint32_t type_id, + PtrAs ptr_as, const spvtools::opt::analysis::Pointer* ptr_ty); /// If `type` is a signed integral, or vector of signed integral,
diff --git a/src/reader/spirv/parser_impl_module_var_test.cc b/src/reader/spirv/parser_impl_module_var_test.cc index 97d2aed..7899896 100644 --- a/src/reader/spirv/parser_impl_module_var_test.cc +++ b/src/reader/spirv/parser_impl_module_var_test.cc
@@ -2413,7 +2413,10 @@ none __ptr_in__u32 { - Identifier[not set]{x_1} + UnaryOp[not set]{ + address-of + Identifier[not set]{x_1} + } } } } @@ -2423,7 +2426,10 @@ none __u32 { - Identifier[not set]{x_11} + UnaryOp[not set]{ + indirection + Identifier[not set]{x_11} + } } } })")) @@ -3295,7 +3301,10 @@ none __ptr_in__u32 { - Identifier[not set]{x_1} + UnaryOp[not set]{ + address-of + Identifier[not set]{x_1} + } } } } @@ -3305,7 +3314,10 @@ none __u32 { - Identifier[not set]{x_11} + UnaryOp[not set]{ + indirection + Identifier[not set]{x_11} + } } } })")) @@ -3614,7 +3626,10 @@ none __ptr_in__u32 { - Identifier[not set]{x_1} + UnaryOp[not set]{ + address-of + Identifier[not set]{x_1} + } } } } @@ -3624,7 +3639,10 @@ none __u32 { - Identifier[not set]{x_11} + UnaryOp[not set]{ + indirection + Identifier[not set]{x_11} + } } } })"))
diff --git a/src/resolver/assignment_validation_test.cc b/src/resolver/assignment_validation_test.cc index 1c5e696..86a92c2 100644 --- a/src/resolver/assignment_validation_test.cc +++ b/src/resolver/assignment_validation_test.cc
@@ -31,39 +31,13 @@ // } auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2)); - auto* lhs = Expr("a"); - auto* rhs = Expr(2.3f); - auto* assign = Assign(Source{{12, 34}}, lhs, rhs); + auto* assign = Assign(Source{{12, 34}}, "a", 2.3f); WrapInFunction(var, assign); ASSERT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - R"(12:34 error: invalid assignment: cannot assign value of type 'f32' to a variable of type 'i32')"); -} - -TEST_F(ResolverAssignmentValidationTest, - AssignThroughPointerWrongeStoreType_Fail) { - // var a : f32; - // let b : ptr<function,f32> = a; - // b = 2; - const auto priv = ast::StorageClass::kFunction; - auto* var_a = Var("a", ty.f32(), priv); - auto* var_b = Const("b", ty.pointer<float>(priv), Expr("a"), {}); - - auto* lhs = Expr("a"); - auto* rhs = Expr(2); - - auto* assign = Assign(Source{{12, 34}}, lhs, rhs); - WrapInFunction(var_a, var_b, assign); - - ASSERT_FALSE(r()->Resolve()); - - EXPECT_EQ( - r()->error(), - R"(12:34 error: invalid assignment: cannot assign value of type 'i32' to a variable of type 'f32')"); + EXPECT_EQ(r()->error(), "12:34 error: cannot assign 'f32' to 'i32'"); } TEST_F(ResolverAssignmentValidationTest, @@ -73,11 +47,7 @@ // a = 2 // } auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2)); - auto* lhs = Expr("a"); - auto* rhs = Expr(2); - - auto* body = Block(Decl(var), Assign(Source{{12, 34}}, lhs, rhs)); - WrapInFunction(body); + WrapInFunction(var, Assign("a", 2)); ASSERT_TRUE(r()->Resolve()) << r()->error(); } @@ -90,17 +60,11 @@ // } auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2)); - auto* lhs = Expr("a"); - auto* rhs = Expr(2.3f); - - auto* block = Block(Decl(var), Assign(Source{{12, 34}}, lhs, rhs)); - WrapInFunction(block); + WrapInFunction(var, Assign(Source{{12, 34}}, "a", 2.3f)); ASSERT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - R"(12:34 error: invalid assignment: cannot assign value of type 'f32' to a variable of type 'i32')"); + EXPECT_EQ(r()->error(), "12:34 error: cannot assign 'f32' to 'i32'"); } TEST_F(ResolverAssignmentValidationTest, @@ -113,20 +77,13 @@ // } auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2)); - auto* lhs = Expr("a"); - auto* rhs = Expr(2.3f); - - auto* inner_block = Block(Decl(var), Assign(Source{{12, 34}}, lhs, rhs)); - + auto* inner_block = Block(Decl(var), Assign(Source{{12, 34}}, "a", 2.3f)); auto* outer_block = Block(inner_block); - WrapInFunction(outer_block); ASSERT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - R"(12:34 error: invalid assignment: cannot assign value of type 'f32' to a variable of type 'i32')"); + EXPECT_EQ(r()->error(), "12:34 error: cannot assign 'f32' to 'i32'"); } TEST_F(ResolverAssignmentValidationTest, AssignToScalar_Fail) { @@ -134,28 +91,17 @@ // 1 = my_var; auto* var = Var("my_var", ty.i32(), ast::StorageClass::kNone, Expr(2)); - auto* lhs = Expr(1); - auto* rhs = Expr("my_var"); - - auto* assign = Assign(Source{{12, 34}}, lhs, rhs); - WrapInFunction(Decl(var), assign); + WrapInFunction(var, Assign(Expr(Source{{12, 34}}, 1), "my_var")); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error v-000x: invalid assignment: left-hand-side does not " - "reference storage: i32"); + EXPECT_EQ(r()->error(), "12:34 error: cannot assign to value of type 'i32'"); } TEST_F(ResolverAssignmentValidationTest, AssignCompatibleTypes_Pass) { - // var a :i32 = 2; + // var a : i32 = 2; // a = 2 auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2)); - - auto* lhs = Expr("a"); - auto* rhs = Expr(2); - - auto* assign = Assign(Source{Source::Location{12, 34}}, lhs, rhs); - WrapInFunction(Decl(var), assign); + WrapInFunction(var, Assign(Source{{12, 34}}, "a", 2)); EXPECT_TRUE(r()->Resolve()) << r()->error(); } @@ -163,17 +109,12 @@ TEST_F(ResolverAssignmentValidationTest, AssignCompatibleTypesThroughAlias_Pass) { // alias myint = i32; - // var a :myint = 2; + // var a : myint = 2; // a = 2 auto* myint = ty.alias("myint", ty.i32()); AST().AddConstructedType(myint); auto* var = Var("a", myint, ast::StorageClass::kNone, Expr(2)); - - auto* lhs = Expr("a"); - auto* rhs = Expr(2); - - auto* assign = Assign(Source{Source::Location{12, 34}}, lhs, rhs); - WrapInFunction(Decl(var), assign); + WrapInFunction(var, Assign(Source{{12, 34}}, "a", 2)); EXPECT_TRUE(r()->Resolve()) << r()->error(); } @@ -185,29 +126,19 @@ // a = b; auto* var_a = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2)); auto* var_b = Var("b", ty.i32(), ast::StorageClass::kNone, Expr(3)); - - auto* lhs = Expr("a"); - auto* rhs = Expr("b"); - - auto* assign = Assign(Source{Source::Location{12, 34}}, lhs, rhs); - WrapInFunction(Decl(var_a), Decl(var_b), assign); + WrapInFunction(var_a, var_b, Assign(Source{{12, 34}}, "a", "b")); EXPECT_TRUE(r()->Resolve()) << r()->error(); } TEST_F(ResolverAssignmentValidationTest, AssignThroughPointer_Pass) { - // var a :i32; - // let b : ptr<function,i32> = a; - // b = 2; + // var a : i32; + // let b : ptr<function,i32> = &a; + // *b = 2; const auto func = ast::StorageClass::kFunction; auto* var_a = Var("a", ty.i32(), func, Expr(2), {}); - auto* var_b = Const("b", ty.pointer<int>(func), Expr("a"), {}); - - auto* lhs = Expr("b"); - auto* rhs = Expr(2); - - auto* assign = Assign(Source{Source::Location{12, 34}}, lhs, rhs); - WrapInFunction(Decl(var_a), Decl(var_b), assign); + auto* var_b = Const("b", ty.pointer<int>(func), AddressOf(Expr("a")), {}); + WrapInFunction(var_a, var_b, Assign(Source{{12, 34}}, Deref("b"), 2)); EXPECT_TRUE(r()->Resolve()) << r()->error(); } @@ -218,21 +149,13 @@ // a = 2 // } auto* var = Const("a", ty.i32(), Expr(2)); - - auto* lhs = Expr("a"); - auto* rhs = Expr(2); - - auto* body = - Block(Decl(var), Assign(Source{Source::Location{12, 34}}, lhs, rhs)); - - WrapInFunction(body); + WrapInFunction(var, Assign(Expr(Source{{12, 34}}, "a"), 2)); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error v-0021: cannot re-assign a constant: 'a'"); + EXPECT_EQ(r()->error(), "12:34 error: cannot assign to value of type 'i32'"); } -TEST_F(ResolverAssignmentValidationTest, AssignFromPointer_Fail) { +TEST_F(ResolverAssignmentValidationTest, AssignNonStorable_Fail) { // var a : [[access(read)]] texture_storage_1d<rgba8unorm>; // var b : [[access(read)]] texture_storage_1d<rgba8unorm>; // a = b; @@ -243,24 +166,23 @@ return ty.access(ast::AccessControl::kReadOnly, tex_type); }; - auto* var_a = Global("a", make_type(), ast::StorageClass::kNone, nullptr, - { - create<ast::BindingDecoration>(0), - create<ast::GroupDecoration>(0), - }); - auto* var_b = Global("b", make_type(), ast::StorageClass::kNone, nullptr, - { - create<ast::BindingDecoration>(1), - create<ast::GroupDecoration>(0), - }); + Global("a", make_type(), ast::StorageClass::kNone, nullptr, + { + create<ast::BindingDecoration>(0), + create<ast::GroupDecoration>(0), + }); + Global("b", make_type(), ast::StorageClass::kNone, nullptr, + { + create<ast::BindingDecoration>(1), + create<ast::GroupDecoration>(0), + }); - WrapInFunction(Assign(Source{{12, 34}}, var_a, var_b)); + WrapInFunction(Assign("a", Expr(Source{{12, 34}}, "b"))); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error v-000x: invalid assignment: right-hand-side is not " - "storable: ptr<uniform_constant, texture_storage_1d<rgba8unorm, " - "read_only>>"); + EXPECT_EQ( + r()->error(), + R"(12:34 error: '[[access(read)]] texture_storage_1d<rgba8unorm>' is not storable)"); } } // namespace
diff --git a/src/resolver/builtins_validation_test.cc b/src/resolver/builtins_validation_test.cc index 363b35a..fe2b59d 100644 --- a/src/resolver/builtins_validation_test.cc +++ b/src/resolver/builtins_validation_test.cc
@@ -108,7 +108,7 @@ TEST_F(ResolverBuiltinsValidationTest, Frexp_Scalar) { auto* a = Var("a", ty.i32()); - auto* builtin = Call("frexp", 1.0f, Expr("a")); + auto* builtin = Call("frexp", 1.0f, AddressOf(Expr("a"))); WrapInFunction(Decl(a), builtin); EXPECT_TRUE(r()->Resolve()) << r()->error(); @@ -118,10 +118,8 @@ TEST_F(ResolverBuiltinsValidationTest, Frexp_Vec2) { auto* a = Var("a", ty.vec2<int>()); - auto* b = Const("b", ty.pointer(ty.vec2<i32>(), ast::StorageClass::kFunction), - Expr("a"), {}); - auto* builtin = Call("frexp", vec2<f32>(1.0f, 1.0f), Expr("b")); - WrapInFunction(Decl(a), Decl(b), builtin); + auto* builtin = Call("frexp", vec2<f32>(1.0f, 1.0f), AddressOf(Expr("a"))); + WrapInFunction(Decl(a), builtin); EXPECT_TRUE(r()->Resolve()) << r()->error(); EXPECT_TRUE(TypeOf(builtin)->is_float_vector()); @@ -130,10 +128,9 @@ TEST_F(ResolverBuiltinsValidationTest, Frexp_Vec3) { auto* a = Var("a", ty.vec3<int>()); - auto* b = Const("b", ty.pointer(ty.vec3<i32>(), ast::StorageClass::kFunction), - Expr("a"), {}); - auto* builtin = Call("frexp", vec3<f32>(1.0f, 1.0f, 1.0f), Expr("b")); - WrapInFunction(Decl(a), Decl(b), builtin); + auto* builtin = + Call("frexp", vec3<f32>(1.0f, 1.0f, 1.0f), AddressOf(Expr("a"))); + WrapInFunction(Decl(a), builtin); EXPECT_TRUE(r()->Resolve()) << r()->error(); EXPECT_TRUE(TypeOf(builtin)->is_float_vector()); @@ -142,10 +139,9 @@ TEST_F(ResolverBuiltinsValidationTest, Frexp_Vec4) { auto* a = Var("a", ty.vec4<int>()); - auto* b = Const("b", ty.pointer(ty.vec4<i32>(), ast::StorageClass::kFunction), - Expr("a"), {}); - auto* builtin = Call("frexp", vec4<f32>(1.0f, 1.0f, 1.0f, 1.0f), Expr("b")); - WrapInFunction(Decl(a), Decl(b), builtin); + auto* builtin = + Call("frexp", vec4<f32>(1.0f, 1.0f, 1.0f, 1.0f), AddressOf(Expr("a"))); + WrapInFunction(Decl(a), builtin); EXPECT_TRUE(r()->Resolve()) << r()->error(); EXPECT_TRUE(TypeOf(builtin)->is_float_vector()); @@ -154,10 +150,8 @@ TEST_F(ResolverBuiltinsValidationTest, Modf_Scalar) { auto* a = Var("a", ty.f32()); - auto* b = - Const("b", ty.pointer<f32>(ast::StorageClass::kFunction), Expr("a"), {}); - auto* builtin = Call("modf", 1.0f, Expr("b")); - WrapInFunction(Decl(a), Decl(b), builtin); + auto* builtin = Call("modf", 1.0f, AddressOf(Expr("a"))); + WrapInFunction(Decl(a), builtin); EXPECT_TRUE(r()->Resolve()) << r()->error(); EXPECT_TRUE(TypeOf(builtin)->Is<sem::F32>()); @@ -166,10 +160,8 @@ TEST_F(ResolverBuiltinsValidationTest, Modf_Vec2) { auto* a = Var("a", ty.vec2<f32>()); - auto* b = Const("b", ty.pointer(ty.vec2<f32>(), ast::StorageClass::kFunction), - Expr("a"), {}); - auto* builtin = Call("modf", vec2<f32>(1.0f, 1.0f), Expr("b")); - WrapInFunction(Decl(a), Decl(b), builtin); + auto* builtin = Call("modf", vec2<f32>(1.0f, 1.0f), AddressOf(Expr("a"))); + WrapInFunction(Decl(a), builtin); EXPECT_TRUE(r()->Resolve()) << r()->error(); EXPECT_TRUE(TypeOf(builtin)->is_float_vector()); @@ -178,10 +170,9 @@ TEST_F(ResolverBuiltinsValidationTest, Modf_Vec3) { auto* a = Var("a", ty.vec3<f32>()); - auto* b = Const("b", ty.pointer(ty.vec3<f32>(), ast::StorageClass::kFunction), - Expr("a"), {}); - auto* builtin = Call("modf", vec3<f32>(1.0f, 1.0f, 1.0f), Expr("b")); - WrapInFunction(Decl(a), Decl(b), builtin); + auto* builtin = + Call("modf", vec3<f32>(1.0f, 1.0f, 1.0f), AddressOf(Expr("a"))); + WrapInFunction(Decl(a), builtin); EXPECT_TRUE(r()->Resolve()) << r()->error(); EXPECT_TRUE(TypeOf(builtin)->is_float_vector()); @@ -190,10 +181,9 @@ TEST_F(ResolverBuiltinsValidationTest, Modf_Vec4) { auto* a = Var("a", ty.vec4<f32>()); - auto* b = Const("b", ty.pointer(ty.vec4<f32>(), ast::StorageClass::kFunction), - Expr("a"), {}); - auto* builtin = Call("modf", vec4<f32>(1.0f, 1.0f, 1.0f, 1.0f), Expr("b")); - WrapInFunction(Decl(a), Decl(b), builtin); + auto* builtin = + Call("modf", vec4<f32>(1.0f, 1.0f, 1.0f, 1.0f), AddressOf(Expr("a"))); + WrapInFunction(Decl(a), builtin); EXPECT_TRUE(r()->Resolve()) << r()->error(); EXPECT_TRUE(TypeOf(builtin)->is_float_vector());
diff --git a/src/resolver/intrinsic_test.cc b/src/resolver/intrinsic_test.cc index af7323d..eb704bf 100644 --- a/src/resolver/intrinsic_test.cc +++ b/src/resolver/intrinsic_test.cc
@@ -182,7 +182,7 @@ EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), "error: no matching call to " + name + - "(ptr<in, f32>, f32)\n\n" + "(f32, f32)\n\n" "2 candidate functions:\n " + name + "(f32) -> bool\n " + name + "(vecN<f32>) -> vecN<bool>\n"); @@ -435,9 +435,8 @@ EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - R"(error: no matching call to dot(ptr<in, vec4<i32>>, ptr<in, vec4<i32>>) + EXPECT_EQ(r()->error(), + R"(error: no matching call to dot(vec4<i32>, vec4<i32>) 1 candidate function: dot(vecN<f32>, vecN<f32>) -> f32 @@ -793,7 +792,7 @@ EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "error: no matching call to arrayLength(ptr<in, array<i32, 4>>)\n\n" + "error: no matching call to arrayLength(array<i32, 4>)\n\n" "1 candidate function:\n" " arrayLength(array<T>) -> u32\n"); } @@ -823,7 +822,7 @@ TEST_F(ResolverIntrinsicDataTest, FrexpScalar) { Global("exp", ty.i32(), ast::StorageClass::kWorkgroup); - auto* call = Call("frexp", 1.0f, "exp"); + auto* call = Call("frexp", 1.0f, AddressOf("exp")); WrapInFunction(call); EXPECT_TRUE(r()->Resolve()) << r()->error(); @@ -834,7 +833,7 @@ TEST_F(ResolverIntrinsicDataTest, FrexpVector) { Global("exp", ty.vec3<i32>(), ast::StorageClass::kWorkgroup); - auto* call = Call("frexp", vec3<f32>(1.0f, 2.0f, 3.0f), "exp"); + auto* call = Call("frexp", vec3<f32>(1.0f, 2.0f, 3.0f), AddressOf("exp")); WrapInFunction(call); EXPECT_TRUE(r()->Resolve()) << r()->error(); @@ -846,7 +845,7 @@ TEST_F(ResolverIntrinsicDataTest, Frexp_Error_FirstParamInt) { Global("exp", ty.i32(), ast::StorageClass::kWorkgroup); - auto* call = Call("frexp", 1, "exp"); + auto* call = Call("frexp", 1, AddressOf("exp")); WrapInFunction(call); EXPECT_FALSE(r()->Resolve()); @@ -861,7 +860,7 @@ TEST_F(ResolverIntrinsicDataTest, Frexp_Error_SecondParamFloatPtr) { Global("exp", ty.f32(), ast::StorageClass::kWorkgroup); - auto* call = Call("frexp", 1.0f, "exp"); + auto* call = Call("frexp", 1.0f, AddressOf("exp")); WrapInFunction(call); EXPECT_FALSE(r()->Resolve()); @@ -890,23 +889,24 @@ TEST_F(ResolverIntrinsicDataTest, Frexp_Error_VectorSizesDontMatch) { Global("exp", ty.vec4<i32>(), ast::StorageClass::kWorkgroup); - auto* call = Call("frexp", vec2<f32>(1.0f, 2.0f), "exp"); + auto* call = Call("frexp", vec2<f32>(1.0f, 2.0f), AddressOf("exp")); WrapInFunction(call); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "error: no matching call to frexp(vec2<f32>, ptr<workgroup, " - "vec4<i32>>)\n\n" - "2 candidate functions:\n" - " frexp(f32, ptr<T>) -> f32 where: T is i32 or u32\n" - " frexp(vecN<f32>, ptr<vecN<T>>) -> vecN<f32> " - "where: T is i32 or u32\n"); + EXPECT_EQ( + r()->error(), + R"(error: no matching call to frexp(vec2<f32>, ptr<workgroup, vec4<i32>>) + +2 candidate functions: + frexp(f32, ptr<T>) -> f32 where: T is i32 or u32 + frexp(vecN<f32>, ptr<vecN<T>>) -> vecN<f32> where: T is i32 or u32 +)"); } TEST_F(ResolverIntrinsicDataTest, ModfScalar) { Global("whole", ty.f32(), ast::StorageClass::kWorkgroup); - auto* call = Call("modf", 1.0f, "whole"); + auto* call = Call("modf", 1.0f, AddressOf("whole")); WrapInFunction(call); EXPECT_TRUE(r()->Resolve()) << r()->error(); @@ -917,7 +917,7 @@ TEST_F(ResolverIntrinsicDataTest, ModfVector) { Global("whole", ty.vec3<f32>(), ast::StorageClass::kWorkgroup); - auto* call = Call("modf", vec3<f32>(1.0f, 2.0f, 3.0f), "whole"); + auto* call = Call("modf", vec3<f32>(1.0f, 2.0f, 3.0f), AddressOf("whole")); WrapInFunction(call); EXPECT_TRUE(r()->Resolve()) << r()->error(); @@ -929,7 +929,7 @@ TEST_F(ResolverIntrinsicDataTest, Modf_Error_FirstParamInt) { Global("whole", ty.f32(), ast::StorageClass::kWorkgroup); - auto* call = Call("modf", 1, "whole"); + auto* call = Call("modf", 1, AddressOf("whole")); WrapInFunction(call); EXPECT_FALSE(r()->Resolve()); @@ -943,7 +943,7 @@ TEST_F(ResolverIntrinsicDataTest, Modf_Error_SecondParamIntPtr) { Global("whole", ty.i32(), ast::StorageClass::kWorkgroup); - auto* call = Call("modf", 1.0f, "whole"); + auto* call = Call("modf", 1.0f, AddressOf("whole")); WrapInFunction(call); EXPECT_FALSE(r()->Resolve()); @@ -970,17 +970,19 @@ TEST_F(ResolverIntrinsicDataTest, Modf_Error_VectorSizesDontMatch) { Global("whole", ty.vec4<f32>(), ast::StorageClass::kWorkgroup); - auto* call = Call("modf", vec2<f32>(1.0f, 2.0f), "whole"); + auto* call = Call("modf", vec2<f32>(1.0f, 2.0f), AddressOf("whole")); WrapInFunction(call); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "error: no matching call to modf(vec2<f32>, ptr<workgroup, " - "vec4<f32>>)\n\n" - "2 candidate functions:\n" - " modf(vecN<f32>, ptr<vecN<f32>>) -> vecN<f32>\n" - " modf(f32, ptr<f32>) -> f32\n"); + EXPECT_EQ( + r()->error(), + R"(error: no matching call to modf(vec2<f32>, ptr<workgroup, vec4<f32>>) + +2 candidate functions: + modf(vecN<f32>, ptr<vecN<f32>>) -> vecN<f32> + modf(f32, ptr<f32>) -> f32 +)"); } using ResolverIntrinsicTest_SingleParam_FloatOrInt = @@ -1652,11 +1654,10 @@ EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - "error: no matching call to determinant(ptr<private, mat2x3<f32>>)\n\n" - "1 candidate function:\n" - " determinant(matNxN<f32>) -> f32\n"); + EXPECT_EQ(r()->error(), + "error: no matching call to determinant(mat2x3<f32>)\n\n" + "1 candidate function:\n" + " determinant(matNxN<f32>) -> f32\n"); } TEST_F(ResolverIntrinsicTest, Determinant_NotMatrix) { @@ -1668,7 +1669,7 @@ EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "error: no matching call to determinant(ptr<private, f32>)\n\n" + "error: no matching call to determinant(f32)\n\n" "1 candidate function:\n" " determinant(matNxN<f32>) -> f32\n"); }
diff --git a/src/resolver/ptr_ref_test.cc b/src/resolver/ptr_ref_test.cc new file mode 100644 index 0000000..e66a0ce --- /dev/null +++ b/src/resolver/ptr_ref_test.cc
@@ -0,0 +1,62 @@ +// 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 "src/resolver/resolver_test_helper.h" +#include "src/sem/reference_type.h" + +#include "gmock/gmock.h" + +namespace tint { +namespace resolver { +namespace { + +struct ResolverPtrRefTest : public resolver::TestHelper, + public testing::Test {}; + +TEST_F(ResolverPtrRefTest, AddressOf) { + // var v : i32; + // &v + + auto* v = Var("v", ty.i32(), ast::StorageClass::kNone); + auto* expr = AddressOf(v); + + WrapInFunction(v, expr); + + EXPECT_TRUE(r()->Resolve()) << r()->error(); + + ASSERT_TRUE(TypeOf(expr)->Is<sem::Pointer>()); + EXPECT_TRUE(TypeOf(expr)->As<sem::Pointer>()->StoreType()->Is<sem::I32>()); + EXPECT_EQ(TypeOf(expr)->As<sem::Pointer>()->StorageClass(), + ast::StorageClass::kFunction); +} + +TEST_F(ResolverPtrRefTest, AddressOfThenDeref) { + // var v : i32; + // *(&v) + + auto* v = Var("v", ty.i32(), ast::StorageClass::kNone); + auto* expr = Deref(AddressOf(v)); + + WrapInFunction(v, expr); + + EXPECT_TRUE(r()->Resolve()) << r()->error(); + + ASSERT_TRUE(TypeOf(expr)->Is<sem::Reference>()); + EXPECT_TRUE(TypeOf(expr)->As<sem::Reference>()->StoreType()->Is<sem::I32>()); +} + +} // namespace +} // namespace resolver +} // namespace tint
diff --git a/src/resolver/ptr_ref_validation_test.cc b/src/resolver/ptr_ref_validation_test.cc new file mode 100644 index 0000000..0b2d07d --- /dev/null +++ b/src/resolver/ptr_ref_validation_test.cc
@@ -0,0 +1,82 @@ +// 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 "src/resolver/resolver_test_helper.h" +#include "src/sem/reference_type.h" + +#include "gmock/gmock.h" + +namespace tint { +namespace resolver { +namespace { + +struct ResolverPtrRefValidationTest : public resolver::TestHelper, + public testing::Test {}; + +TEST_F(ResolverPtrRefValidationTest, AddressOfLiteral) { + // &1 + + auto* expr = AddressOf(Expr(Source{{12, 34}}, 1)); + + WrapInFunction(expr); + + EXPECT_FALSE(r()->Resolve()); + + EXPECT_EQ(r()->error(), "12:34 error: cannot take the address of expression"); +} + +TEST_F(ResolverPtrRefValidationTest, AddressOfLet) { + // let l : i32 = 1; + // &l + auto* l = Const("l", ty.i32(), Expr(1)); + auto* expr = AddressOf(Expr(Source{{12, 34}}, "l")); + + WrapInFunction(l, expr); + + EXPECT_FALSE(r()->Resolve()); + + EXPECT_EQ(r()->error(), "12:34 error: cannot take the address of expression"); +} + +TEST_F(ResolverPtrRefValidationTest, DerefOfLiteral) { + // *1 + + auto* expr = Deref(Expr(Source{{12, 34}}, 1)); + + WrapInFunction(expr); + + EXPECT_FALSE(r()->Resolve()); + + EXPECT_EQ(r()->error(), + "12:34 error: cannot dereference expression of type 'i32'"); +} + +TEST_F(ResolverPtrRefValidationTest, DerefOfVar) { + // var v : i32 = 1; + // *1 + auto* v = Var("v", ty.i32()); + auto* expr = Deref(Expr(Source{{12, 34}}, "v")); + + WrapInFunction(v, expr); + + EXPECT_FALSE(r()->Resolve()); + + EXPECT_EQ(r()->error(), + "12:34 error: cannot dereference expression of type 'i32'"); +} + +} // namespace +} // namespace resolver +} // namespace tint
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index 67b0734..a5e3379 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc
@@ -53,6 +53,7 @@ #include "src/sem/member_accessor_expression.h" #include "src/sem/multisampled_texture_type.h" #include "src/sem/pointer_type.h" +#include "src/sem/reference_type.h" #include "src/sem/sampled_texture_type.h" #include "src/sem/sampler_type.h" #include "src/sem/statement.h" @@ -226,22 +227,6 @@ return false; } -bool Resolver::IsValidAssignment(const sem::Type* lhs, const sem::Type* rhs) { - // TODO(crbug.com/tint/659): This is a rough approximation, and is missing - // checks for writability of pointer storage class, access control, etc. - // This will need to be fixed after WGSL agrees the behavior of pointers / - // references. - // Check: - if (lhs->UnwrapAccess() != rhs->UnwrapAccess()) { - // Try RHS dereference - if (lhs->UnwrapAccess() != rhs->UnwrapAll()) { - return false; - } - } - - return true; -} - bool Resolver::ResolveInternal() { Mark(&builder_->AST()); @@ -438,7 +423,7 @@ } Resolver::VariableInfo* Resolver::Variable(ast::Variable* var, - bool is_parameter) { + VariableKind kind) { if (variable_to_info_.count(var)) { TINT_ICE(diagnostics_) << "Variable " << builder_->Symbols().NameFor(var->symbol()) @@ -446,17 +431,21 @@ return nullptr; } - // If the variable has a declared type, resolve it. std::string type_name; - const sem::Type* type = nullptr; + const sem::Type* storage_type = nullptr; + + // If the variable has a declared type, resolve it. if (auto* ty = var->type()) { type_name = ty->FriendlyName(builder_->Symbols()); - type = Type(ty); - if (!type) { + storage_type = Type(ty); + if (!storage_type) { return nullptr; } } + std::string rhs_type_name; + const sem::Type* rhs_type = nullptr; + // Does the variable have a constructor? if (auto* ctor = var->constructor()) { Mark(var->constructor()); @@ -465,32 +454,57 @@ } // Fetch the constructor's type - auto* rhs_type = TypeOf(ctor); + rhs_type_name = TypeNameOf(ctor); + rhs_type = TypeOf(ctor); if (!rhs_type) { return nullptr; } // If the variable has no declared type, infer it from the RHS - if (type == nullptr) { - type_name = TypeNameOf(ctor); - type = rhs_type->UnwrapPtr(); + if (!storage_type) { + type_name = rhs_type_name; + storage_type = rhs_type->UnwrapRef(); // Implicit load of RHS } - - if (!IsValidAssignment(type, rhs_type)) { - diagnostics_.add_error( - "variable of type '" + type_name + - "' cannot be initialized with a value of type '" + - TypeNameOf(ctor) + "'", - var->source()); - return nullptr; - } - } else if (var->is_const() && !is_parameter && + } else if (var->is_const() && kind != VariableKind::kParameter && !ast::HasDecoration<ast::OverrideDecoration>(var->decorations())) { diagnostics_.add_error("let declarations must have initializers", var->source()); return nullptr; } + if (!storage_type) { + TINT_ICE(diagnostics_) + << "failed to determine storage type for variable '" + + builder_->Symbols().NameFor(var->symbol()) + "'\n" + << "Source: " << var->source(); + return nullptr; + } + + auto storage_class = var->declared_storage_class(); + if (storage_class == ast::StorageClass::kNone) { + if (storage_type->UnwrapRef()->is_handle()) { + // https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables + // If the store type is a texture type or a sampler type, then the + // variable declaration must not have a storage class decoration. The + // storage class will always be handle. + storage_class = ast::StorageClass::kUniformConstant; + } else if (kind == VariableKind::kLocal && !var->is_const()) { + storage_class = ast::StorageClass::kFunction; + } + } + + auto* type = storage_type; + if (!var->is_const()) { + // Variable declaration. Unlike `let`, `var` has storage. + // Variables are always of a reference type to the declared storage type. + type = builder_->create<sem::Reference>(storage_type, storage_class); + } + + if (rhs_type && !ValidateVariableConstructor(var, storage_type, type_name, + rhs_type, rhs_type_name)) { + return nullptr; + } + // TODO(crbug.com/tint/802): Temporary while ast::AccessControl exits. auto find_first_access_control = [this](ast::Type* ty) -> ast::AccessControl* { @@ -519,12 +533,39 @@ } auto* info = variable_infos_.Create(var, const_cast<sem::Type*>(type), - type_name, access_control); + type_name, storage_class, access_control); variable_to_info_.emplace(var, info); return info; } +bool Resolver::ValidateVariableConstructor(const ast::Variable* var, + const sem::Type* storage_type, + const std::string& type_name, + const sem::Type* rhs_type, + const std::string& rhs_type_name) { + auto* value_type = rhs_type->UnwrapRef(); // Implicit load of RHS + + // RHS needs to be of a storable type + if (!var->is_const() && !IsStorable(value_type)) { + diagnostics_.add_error( + "'" + rhs_type_name + "' is not storable for assignment", + var->constructor()->source()); + return false; + } + + // Value type has to match storage type + if (storage_type->UnwrapAccess() != value_type->UnwrapAccess()) { + std::string decl = var->is_const() ? "let" : "var"; + diagnostics_.add_error("cannot initialize " + decl + " of type '" + + type_name + "' with value of type '" + + rhs_type_name + "'", + var->source()); + return false; + } + return true; +} + bool Resolver::GlobalVariable(ast::Variable* var) { if (variable_stack_.has(var->symbol())) { diagnostics_.add_error("v-0011", @@ -534,7 +575,7 @@ return false; } - auto* info = Variable(var, /* is_parameter */ false); + auto* info = Variable(var, VariableKind::kGlobal); if (!info) { return false; } @@ -571,8 +612,9 @@ return false; } - if (!ApplyStorageClassUsageToType(info->storage_class, info->type, - var->source())) { + if (!ApplyStorageClassUsageToType( + info->storage_class, const_cast<sem::Type*>(info->type->UnwrapRef()), + var->source())) { diagnostics_.add_note("while instantiating variable " + builder_->Symbols().NameFor(var->symbol()), var->source()); @@ -636,7 +678,8 @@ // attributes. if (!binding_point) { diagnostics_.add_error( - "resource variables require [[group]] and [[binding]] decorations", + "resource variables require [[group]] and [[binding]] " + "decorations", info->declaration->source()); return false; } @@ -666,7 +709,7 @@ // attribute, satisfying the storage class constraints. auto* str = info->access_control != ast::AccessControl::kInvalid - ? info->type->As<sem::Struct>() + ? info->type->UnwrapRef()->As<sem::Struct>() : nullptr; if (!str) { @@ -695,7 +738,7 @@ // A variable in the uniform storage class is a uniform buffer variable. // Its store type must be a host-shareable structure type with block // attribute, satisfying the storage class constraints. - auto* str = info->type->As<sem::Struct>(); + auto* str = info->type->UnwrapRef()->As<sem::Struct>(); if (!str) { diagnostics_.add_error( "variables declared in the <uniform> storage class must be of a " @@ -726,8 +769,8 @@ bool Resolver::ValidateVariable(const VariableInfo* info) { auto* var = info->declaration; - auto* type = info->type; - if (auto* r = type->As<sem::Array>()) { + auto* storage_type = info->type->UnwrapRef(); + if (auto* r = storage_type->As<sem::Array>()) { if (r->IsRuntimeSized()) { diagnostics_.add_error( "v-0015", @@ -737,15 +780,14 @@ } } - if (auto* r = type->As<sem::MultisampledTexture>()) { + if (auto* r = storage_type->As<sem::MultisampledTexture>()) { if (r->dim() != ast::TextureDimension::k2d) { diagnostics_.add_error("Only 2d multisampled textures are supported", var->source()); return false; } - auto* data_type = r->type()->UnwrapAll(); - if (!data_type->is_numeric_scalar()) { + if (!r->type()->UnwrapRef()->is_numeric_scalar()) { diagnostics_.add_error( "texture_multisampled_2d<type>: type must be f32, i32 or u32", var->source()); @@ -753,7 +795,7 @@ } } - if (auto* storage_tex = type->As<sem::StorageTexture>()) { + if (auto* storage_tex = info->type->UnwrapRef()->As<sem::StorageTexture>()) { if (storage_tex->access_control() == ast::AccessControl::kInvalid) { diagnostics_.add_error("Storage Textures must have access control.", var->source()); @@ -786,12 +828,12 @@ } } - if (type->UnwrapAll()->is_handle() && + if (storage_type->is_handle() && var->declared_storage_class() != ast::StorageClass::kNone) { // https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables - // If the store type is a texture type or a sampler type, then the variable - // declaration must not have a storage class decoration. The storage class - // will always be handle. + // If the store type is a texture type or a sampler type, then the + // variable declaration must not have a storage class decoration. The + // storage class will always be handle. diagnostics_.add_error("variables of type '" + info->type_name + "' must not have a storage class", var->source()); @@ -893,10 +935,10 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func, const FunctionInfo* info) { // Use a lambda to validate the entry point decorations for a type. - // Persistent state is used to track which builtins and locations have already - // been seen, in order to catch conflicts. - // TODO(jrprice): This state could be stored in FunctionInfo instead, and then - // passed to sem::Function since it would be useful there too. + // Persistent state is used to track which builtins and locations have + // already been seen, in order to catch conflicts. + // TODO(jrprice): This state could be stored in FunctionInfo instead, and + // then passed to sem::Function since it would be useful there too. std::unordered_set<ast::Builtin> builtins; std::unordered_set<uint32_t> locations; enum class ParamOrRetType { @@ -1147,7 +1189,7 @@ variable_stack_.push_scope(); for (auto* param : func->params()) { Mark(param); - auto* param_info = Variable(param, /* is_parameter */ true); + auto* param_info = Variable(param, VariableKind::kParameter); if (!param_info) { return false; } @@ -1377,7 +1419,7 @@ return false; } - auto* cond_type = TypeOf(stmt->condition())->UnwrapAll(); + auto* cond_type = TypeOf(stmt->condition())->UnwrapRef(); if (cond_type != builder_->ty.bool_()) { diagnostics_.add_error("if statement condition must be bool, got " + cond_type->FriendlyName(builder_->Symbols()), @@ -1409,7 +1451,7 @@ return false; } - auto* else_cond_type = TypeOf(cond)->UnwrapAll(); + auto* else_cond_type = TypeOf(cond)->UnwrapRef(); if (else_cond_type != builder_->ty.bool_()) { diagnostics_.add_error( "else statement condition must be bool, got " + @@ -1525,7 +1567,7 @@ } auto* res = TypeOf(expr->array()); - auto* parent_type = res->UnwrapAll(); + auto* parent_type = res->UnwrapRef(); const sem::Type* ret = nullptr; if (auto* arr = parent_type->As<sem::Array>()) { ret = arr->ElemType(); @@ -1540,15 +1582,9 @@ return false; } - // If we're extracting from a pointer, we return a pointer. - if (auto* ptr = res->As<sem::Pointer>()) { - ret = builder_->create<sem::Pointer>(ret, ptr->StorageClass()); - } else if (auto* arr = parent_type->As<sem::Array>()) { - if (!arr->ElemType()->is_scalar()) { - // If we extract a non-scalar from an array then we also get a pointer. We - // will generate a Function storage class variable to store this into. - ret = builder_->create<sem::Pointer>(ret, ast::StorageClass::kFunction); - } + // If we're extracting from a reference, we return a reference. + if (auto* ref = res->As<sem::Reference>()) { + ret = builder_->create<sem::Reference>(ret, ref->StorageClass()); } SetType(expr, ret); @@ -1569,9 +1605,9 @@ return false; } - // The expression has to be an identifier as you can't store function pointers - // but, if it isn't we'll just use the normal result determination to be on - // the safe side. + // The expression has to be an identifier as you can't store function + // pointers but, if it isn't we'll just use the normal result determination + // to be on the safe side. Mark(call->func()); auto* ident = call->func()->As<ast::IdentifierExpression>(); if (!ident) { @@ -1605,7 +1641,8 @@ auto* callee_func = callee_func_it->second; // Note: Requires called functions to be resolved first. - // This is currently guaranteed as functions must be declared before use. + // This is currently guaranteed as functions must be declared before + // use. current_function_->transitive_calls.add(callee_func); for (auto* transitive_call : callee_func->transitive_calls) { current_function_->transitive_calls.add(transitive_call); @@ -1692,10 +1729,10 @@ const ast::TypeConstructorExpression* ctor, const sem::Vector* vec_type) { auto& values = ctor->values(); - auto* elem_type = vec_type->type()->UnwrapAll(); + auto* elem_type = vec_type->type(); size_t value_cardinality_sum = 0; for (auto* value : values) { - auto* value_type = TypeOf(value)->UnwrapAll(); + auto* value_type = TypeOf(value)->UnwrapRef(); if (value_type->is_scalar()) { if (elem_type != value_type) { diagnostics_.add_error( @@ -1709,7 +1746,7 @@ value_cardinality_sum++; } else if (auto* value_vec = value_type->As<sem::Vector>()) { - auto* value_elem_type = value_vec->type()->UnwrapAll(); + auto* value_elem_type = value_vec->type(); // A mismatch of vector type parameter T is only an error if multiple // arguments are present. A single argument constructor constitutes a // type conversion expression. @@ -1766,7 +1803,7 @@ return true; } - auto* elem_type = matrix_type->type()->UnwrapAll(); + auto* elem_type = matrix_type->type(); if (matrix_type->columns() != values.size()) { const Source& values_start = values[0]->source(); const Source& values_end = values[values.size() - 1]->source(); @@ -1780,11 +1817,11 @@ } for (auto* value : values) { - auto* value_type = TypeOf(value)->UnwrapAll(); + auto* value_type = TypeOf(value)->UnwrapRef(); auto* value_vec = value_type->As<sem::Vector>(); if (!value_vec || value_vec->size() != matrix_type->rows() || - elem_type != value_vec->type()->UnwrapAll()) { + elem_type != value_vec->type()) { diagnostics_.add_error("expected argument type '" + VectorPretty(matrix_type->rows(), elem_type) + "' in '" + TypeNameOf(ctor) + @@ -1802,26 +1839,15 @@ auto symbol = expr->symbol(); VariableInfo* var; if (variable_stack_.get(symbol, &var)) { - // A constant is the type, but a variable is always a pointer so synthesize - // the pointer around the variable type. - if (var->declaration->is_const()) { - SetType(expr, var->type, var->type_name); - } else if (var->type->Is<sem::Pointer>()) { - SetType(expr, var->type, var->type_name); - } else { - SetType(expr, - builder_->create<sem::Pointer>(const_cast<sem::Type*>(var->type), - var->storage_class), - var->type_name); - } + SetType(expr, var->type, var->type_name); var->users.push_back(expr); set_referenced_from_function_if_needed(var, true); if (current_block_) { - // If identifier is part of a loop continuing block, make sure it doesn't - // refer to a variable that is bypassed by a continue statement in the - // loop's body block. + // If identifier is part of a loop continuing block, make sure it + // doesn't refer to a variable that is bypassed by a continue statement + // in the loop's body block. if (auto* continuing_block = current_block_->FindFirstParent( sem::BlockStatement::Type::kLoopContinuing)) { auto* loop_block = @@ -1878,13 +1904,13 @@ return false; } - auto* res = TypeOf(expr->structure()); - auto* data_type = res->UnwrapAll(); + auto* structure = TypeOf(expr->structure()); + auto* storage_type = structure->UnwrapRef(); sem::Type* ret = nullptr; std::vector<uint32_t> swizzle; - if (auto* str = data_type->As<sem::Struct>()) { + if (auto* str = storage_type->As<sem::Struct>()) { Mark(expr->member()); auto symbol = expr->member()->symbol(); @@ -1904,14 +1930,14 @@ return false; } - // If we're extracting from a pointer, we return a pointer. - if (auto* ptr = res->As<sem::Pointer>()) { - ret = builder_->create<sem::Pointer>(ret, ptr->StorageClass()); + // If we're extracting from a reference, we return a reference. + if (auto* ref = structure->As<sem::Reference>()) { + ret = builder_->create<sem::Reference>(ret, ref->StorageClass()); } builder_->Sem().Add(expr, builder_->create<sem::StructMemberAccess>( expr, ret, current_statement_, member)); - } else if (auto* vec = data_type->As<sem::Vector>()) { + } else if (auto* vec = storage_type->As<sem::Vector>()) { Mark(expr->member()); std::string s = builder_->Symbols().NameFor(expr->member()->symbol()); auto size = s.size(); @@ -1967,9 +1993,9 @@ if (size == 1) { // A single element swizzle is just the type of the vector. ret = vec->type(); - // If we're extracting from a pointer, we return a pointer. - if (auto* ptr = res->As<sem::Pointer>()) { - ret = builder_->create<sem::Pointer>(ret, ptr->StorageClass()); + // If we're extracting from a reference, we return a reference. + if (auto* ref = structure->As<sem::Reference>()) { + ret = builder_->create<sem::Reference>(ret, ref->StorageClass()); } } else { // The vector will have a number of components equal to the length of @@ -1983,7 +2009,7 @@ } else { diagnostics_.add_error( "invalid use of member accessor on a non-vector/non-struct " + - data_type->type_name(), + TypeNameOf(expr->structure()), expr->source()); return false; } @@ -2001,8 +2027,8 @@ using Matrix = sem::Matrix; using Vector = sem::Vector; - auto* lhs_type = const_cast<sem::Type*>(TypeOf(expr->lhs())->UnwrapAll()); - auto* rhs_type = const_cast<sem::Type*>(TypeOf(expr->rhs())->UnwrapAll()); + auto* lhs_type = const_cast<sem::Type*>(TypeOf(expr->lhs())->UnwrapRef()); + auto* rhs_type = const_cast<sem::Type*>(TypeOf(expr->rhs())->UnwrapRef()); auto* lhs_vec = lhs_type->As<Vector>(); auto* lhs_vec_elem_type = lhs_vec ? lhs_vec->type() : nullptr; @@ -2169,7 +2195,7 @@ if (expr->IsAnd() || expr->IsOr() || expr->IsXor() || expr->IsShiftLeft() || expr->IsShiftRight() || expr->IsAdd() || expr->IsSubtract() || expr->IsDivide() || expr->IsModulo()) { - SetType(expr, TypeOf(expr->lhs())->UnwrapPtr()); + SetType(expr, TypeOf(expr->lhs())->UnwrapRef()); return true; } // Result type is a scalar or vector of boolean type @@ -2177,7 +2203,7 @@ expr->IsNotEqual() || expr->IsLessThan() || expr->IsGreaterThan() || expr->IsLessThanEqual() || expr->IsGreaterThanEqual()) { auto* bool_type = builder_->create<sem::Bool>(); - auto* param_type = TypeOf(expr->lhs())->UnwrapAll(); + auto* param_type = TypeOf(expr->lhs())->UnwrapRef(); sem::Type* result_type = bool_type; if (auto* vec = param_type->As<sem::Vector>()) { result_type = builder_->create<sem::Vector>(bool_type, vec->size()); @@ -2186,8 +2212,8 @@ return true; } if (expr->IsMultiply()) { - auto* lhs_type = TypeOf(expr->lhs())->UnwrapAll(); - auto* rhs_type = TypeOf(expr->rhs())->UnwrapAll(); + auto* lhs_type = TypeOf(expr->lhs())->UnwrapRef(); + auto* rhs_type = TypeOf(expr->rhs())->UnwrapRef(); // Note, the ordering here matters. The later checks depend on the prior // checks having been done. @@ -2234,16 +2260,55 @@ return false; } -bool Resolver::UnaryOp(ast::UnaryOpExpression* expr) { - Mark(expr->expr()); +bool Resolver::UnaryOp(ast::UnaryOpExpression* unary) { + Mark(unary->expr()); - // Result type matches the parameter type. - if (!Expression(expr->expr())) { + // Resolve the inner expression + if (!Expression(unary->expr())) { return false; } - auto* result_type = TypeOf(expr->expr())->UnwrapPtr(); - SetType(expr, result_type); + auto* expr_type = TypeOf(unary->expr()); + if (!expr_type) { + return false; + } + + std::string type_name; + const sem::Type* type = nullptr; + + switch (unary->op()) { + case ast::UnaryOp::kNegation: + case ast::UnaryOp::kNot: + // Result type matches the deref'd inner type. + type_name = TypeNameOf(unary->expr()); + type = expr_type->UnwrapRef(); + break; + + case ast::UnaryOp::kAddressOf: + if (auto* ref = expr_type->As<sem::Reference>()) { + type = builder_->create<sem::Pointer>(ref->StoreType(), + ref->StorageClass()); + } else { + diagnostics_.add_error("cannot take the address of expression", + unary->expr()->source()); + return false; + } + break; + + case ast::UnaryOp::kIndirection: + if (auto* ptr = expr_type->As<sem::Pointer>()) { + type = builder_->create<sem::Reference>(ptr->StoreType(), + ptr->StorageClass()); + } else { + diagnostics_.add_error("cannot dereference expression of type '" + + TypeNameOf(unary->expr()) + "'", + unary->expr()->source()); + return false; + } + break; + } + + SetType(unary, type); return true; } @@ -2257,11 +2322,11 @@ diagnostics_.add_error(error_code, "redeclared identifier '" + builder_->Symbols().NameFor(var->symbol()) + "'", - stmt->source()); + var->source()); return false; } - auto* info = Variable(var, /* is_parameter */ false); + auto* info = Variable(var, VariableKind::kLocal); if (!info) { return false; } @@ -2357,8 +2422,8 @@ void Resolver::CreateSemanticNodes() const { auto& sem = builder_->Sem(); - // Collate all the 'ancestor_entry_points' - this is a map of function symbol - // to all the entry points that transitively call the function. + // Collate all the 'ancestor_entry_points' - this is a map of function + // symbol to all the entry points that transitively call the function. std::unordered_map<Symbol, std::vector<Symbol>> ancestor_entry_points; for (auto* func : builder_->AST().Functions()) { auto it = function_to_info_.find(func); @@ -2641,7 +2706,7 @@ bool Resolver::ValidateStructure(const sem::Struct* str) { for (auto* member : str->Members()) { - if (auto* r = member->Type()->UnwrapAll()->As<sem::Array>()) { + if (auto* r = member->Type()->As<sem::Array>()) { if (r->IsRuntimeSized()) { if (member != str->Members().back()) { diagnostics_.add_error( @@ -2738,8 +2803,8 @@ for (auto* deco : member->decorations()) { Mark(deco); if (auto* o = deco->As<ast::StructMemberOffsetDecoration>()) { - // Offset decorations are not part of the WGSL spec, but are emitted by - // the SPIR-V reader. + // Offset decorations are not part of the WGSL spec, but are emitted + // by the SPIR-V reader. if (o->offset() < struct_size) { diagnostics_.add_error("offsets must be in ascending order", o->source()); @@ -2805,10 +2870,10 @@ bool Resolver::ValidateReturn(const ast::ReturnStatement* ret) { auto* func_type = current_function_->return_type; - auto* ret_type = ret->has_value() ? TypeOf(ret->value())->UnwrapAll() + auto* ret_type = ret->has_value() ? TypeOf(ret->value())->UnwrapRef() : builder_->ty.void_(); - if (func_type->UnwrapAll() != ret_type) { + if (func_type->UnwrapRef() != ret_type) { diagnostics_.add_error("v-000y", "return statement type must match its function " "return type, returned '" + @@ -2828,8 +2893,8 @@ if (auto* value = ret->value()) { Mark(value); - // Validate after processing the return value expression so that its type is - // available for validation + // Validate after processing the return value expression so that its type + // is available for validation return Expression(value) && ValidateReturn(ret); } @@ -2837,7 +2902,7 @@ } bool Resolver::ValidateSwitch(const ast::SwitchStatement* s) { - auto* cond_type = TypeOf(s->condition())->UnwrapAll(); + auto* cond_type = TypeOf(s->condition())->UnwrapRef(); if (!cond_type->is_integer_scalar()) { diagnostics_.add_error("v-0025", "switch statement selector expression must be of a " @@ -2928,67 +2993,6 @@ return true; } -bool Resolver::ValidateAssignment(const ast::AssignmentStatement* a) { - auto* lhs = a->lhs(); - auto* rhs = a->rhs(); - - // TODO(crbug.com/tint/659): This logic needs updating once pointers are - // pinned down in the WGSL spec. - auto* lhs_type = TypeOf(lhs)->UnwrapAll(); - auto* rhs_type = TypeOf(rhs); - if (!IsValidAssignment(lhs_type, rhs_type)) { - diagnostics_.add_error("invalid assignment: cannot assign value of type '" + - rhs_type->FriendlyName(builder_->Symbols()) + - "' to a variable of type '" + - lhs_type->FriendlyName(builder_->Symbols()) + - "'", - a->source()); - return false; - } - - // Pointers are not storable in WGSL, but the right-hand side must be - // storable. The raw right-hand side might be a pointer value which must be - // loaded (dereferenced) to provide the value to be stored. - auto* rhs_result_type = TypeOf(rhs)->UnwrapAll(); - if (!IsStorable(rhs_result_type)) { - diagnostics_.add_error( - "v-000x", - "invalid assignment: right-hand-side is not storable: " + - TypeOf(rhs)->FriendlyName(builder_->Symbols()), - a->source()); - return false; - } - - // lhs must be a pointer or a constant - auto* lhs_result_type = TypeOf(lhs)->UnwrapAccess(); - if (!lhs_result_type->Is<sem::Pointer>()) { - // In case lhs is a constant identifier, output a nicer message as it's - // likely to be a common programmer error. - if (auto* ident = lhs->As<ast::IdentifierExpression>()) { - VariableInfo* var; - if (variable_stack_.get(ident->symbol(), &var) && - var->declaration->is_const()) { - diagnostics_.add_error( - "v-0021", - "cannot re-assign a constant: '" + - builder_->Symbols().NameFor(ident->symbol()) + "'", - a->source()); - return false; - } - } - - // Issue a generic error. - diagnostics_.add_error( - "v-000x", - "invalid assignment: left-hand-side does not reference storage: " + - TypeOf(lhs)->FriendlyName(builder_->Symbols()), - a->source()); - return false; - } - - return true; -} - bool Resolver::Assignment(ast::AssignmentStatement* a) { Mark(a->lhs()); Mark(a->rhs()); @@ -2999,10 +3003,52 @@ return ValidateAssignment(a); } +bool Resolver::ValidateAssignment(const ast::AssignmentStatement* a) { + // https://gpuweb.github.io/gpuweb/wgsl/#assignment-statement + auto const* lhs_type = TypeOf(a->lhs()); + auto const* rhs_type = TypeOf(a->rhs()); + + auto* lhs_ref = lhs_type->As<sem::Reference>(); + if (!lhs_ref) { + // LHS is not a reference, so it has no storage. + diagnostics_.add_error( + "cannot assign to value of type '" + TypeNameOf(a->lhs()) + "'", + a->lhs()->source()); + + return false; + } + + auto* storage_type_with_access = lhs_ref->StoreType(); + + // TODO(crbug.com/tint/809): The originating variable of the left-hand side + // must not have an access(read) access attribute. + // https://gpuweb.github.io/gpuweb/wgsl/#assignment + + auto* storage_type = storage_type_with_access->UnwrapAccess(); + auto* value_type = rhs_type->UnwrapRef(); // Implicit load of RHS + + // RHS needs to be of a storable type + if (!IsStorable(value_type)) { + diagnostics_.add_error("'" + TypeNameOf(a->rhs()) + "' is not storable", + a->rhs()->source()); + return false; + } + + // Value type has to match storage type + if (storage_type != value_type) { + diagnostics_.add_error("cannot assign '" + TypeNameOf(a->rhs()) + "' to '" + + TypeNameOf(a->lhs()) + "'", + a->source()); + return false; + } + + return true; +} + bool Resolver::ApplyStorageClassUsageToType(ast::StorageClass sc, sem::Type* ty, const Source& usage) { - ty = const_cast<sem::Type*>(ty->UnwrapAccess()); + ty = const_cast<sem::Type*>(ty->UnwrapRef()); if (auto* str = ty->As<sem::Struct>()) { if (str->StorageClassUsage().count(sc)) { @@ -3079,23 +3125,15 @@ } Resolver::VariableInfo::VariableInfo(const ast::Variable* decl, - sem::Type* ctype, + sem::Type* ty, const std::string& tn, + ast::StorageClass sc, ast::AccessControl::Access ac) : declaration(decl), - type(ctype), + type(ty), type_name(tn), - storage_class(decl->declared_storage_class()), - access_control(ac) { - if (storage_class == ast::StorageClass::kNone && - type->UnwrapAll()->is_handle()) { - // https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables - // If the store type is a texture type or a sampler type, then the variable - // declaration must not have a storage class decoration. The storage class - // will always be handle. - storage_class = ast::StorageClass::kUniformConstant; - } -} + storage_class(sc), + access_control(ac) {} Resolver::VariableInfo::~VariableInfo() = default;
diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h index 3b8b05e..76a722f 100644 --- a/src/resolver/resolver.h +++ b/src/resolver/resolver.h
@@ -79,13 +79,6 @@ /// @returns true if the given type is host-shareable bool IsHostShareable(const sem::Type* type); - /// @param lhs the assignment store type (non-pointer) - /// @param rhs the assignment source type (non-pointer or pointer with - /// auto-deref) - /// @returns true an expression of type `rhs` can be assigned to a variable, - /// structure member or array element of type `lhs` - static bool IsValidAssignment(const sem::Type* lhs, const sem::Type* rhs); - private: /// Structure holding semantic information about a variable. /// Used to build the sem::Variable nodes at the end of resolving. @@ -93,6 +86,7 @@ VariableInfo(const ast::Variable* decl, sem::Type* type, const std::string& type_name, + ast::StorageClass storage_class, ast::AccessControl::Access ac); ~VariableInfo(); @@ -139,6 +133,43 @@ sem::Statement* statement; }; + /// Structure holding semantic information about a block (i.e. scope), such as + /// parent block and variables declared in the block. + /// Used to validate variable scoping rules. + struct BlockInfo { + enum class Type { kGeneric, kLoop, kLoopContinuing, kSwitchCase }; + + BlockInfo(const ast::BlockStatement* block, Type type, BlockInfo* parent); + ~BlockInfo(); + + template <typename Pred> + BlockInfo* FindFirstParent(Pred&& pred) { + BlockInfo* curr = this; + while (curr && !pred(curr)) { + curr = curr->parent; + } + return curr; + } + + BlockInfo* FindFirstParent(BlockInfo::Type ty) { + return FindFirstParent( + [ty](auto* block_info) { return block_info->type == ty; }); + } + + ast::BlockStatement const* const block; + Type const type; + BlockInfo* const parent; + std::vector<const ast::Variable*> decls; + + // first_continue is set to the index of the first variable in decls + // declared after the first continue statement in a loop block, if any. + constexpr static size_t kNoContinue = size_t(~0); + size_t first_continue = kNoContinue; + }; + + /// Describes the context in which a variable is declared + enum class VariableKind { kParameter, kLocal, kGlobal }; + /// Resolves the program, without creating final the semantic nodes. /// @returns true on success, false on error bool ResolveInternal(); @@ -207,6 +238,11 @@ bool ValidateStructure(const sem::Struct* str); bool ValidateSwitch(const ast::SwitchStatement* s); bool ValidateVariable(const VariableInfo* info); + bool ValidateVariableConstructor(const ast::Variable* var, + const sem::Type* storage_type, + const std::string& type_name, + const sem::Type* rhs_type, + const std::string& rhs_type_name); bool ValidateVectorConstructor(const ast::TypeConstructorExpression* ctor, const sem::Vector* vec_type); @@ -235,8 +271,8 @@ /// @note this method does not resolve the decorations as these are /// context-dependent (global, local, parameter) /// @param var the variable to create or return the `VariableInfo` for - /// @param is_parameter true if the variable represents a parameter - VariableInfo* Variable(ast::Variable* var, bool is_parameter); + /// @param kind what kind of variable we are declaring + VariableInfo* Variable(ast::Variable* var, VariableKind kind); /// Records the storage class usage for the given type, and any transient /// dependencies of the type. Validates that the type can be used for the
diff --git a/src/resolver/resolver_test.cc b/src/resolver/resolver_test.cc index 330492e..e81be39 100644 --- a/src/resolver/resolver_test.cc +++ b/src/resolver/resolver_test.cc
@@ -36,6 +36,7 @@ #include "src/sem/call.h" #include "src/sem/function.h" #include "src/sem/member_accessor_expression.h" +#include "src/sem/reference_type.h" #include "src/sem/sampled_texture_type.h" #include "src/sem/statement.h" #include "src/sem/variable.h" @@ -66,7 +67,7 @@ ASSERT_NE(TypeOf(lhs), nullptr); ASSERT_NE(TypeOf(rhs), nullptr); - EXPECT_TRUE(TypeOf(lhs)->UnwrapAll()->Is<sem::F32>()); + EXPECT_TRUE(TypeOf(lhs)->UnwrapRef()->Is<sem::F32>()); EXPECT_TRUE(TypeOf(rhs)->Is<sem::F32>()); EXPECT_EQ(StmtOf(lhs), assign); EXPECT_EQ(StmtOf(rhs), assign); @@ -90,7 +91,7 @@ ASSERT_NE(TypeOf(lhs), nullptr); ASSERT_NE(TypeOf(rhs), nullptr); - EXPECT_TRUE(TypeOf(lhs)->UnwrapAll()->Is<sem::F32>()); + EXPECT_TRUE(TypeOf(lhs)->UnwrapRef()->Is<sem::F32>()); EXPECT_TRUE(TypeOf(rhs)->Is<sem::F32>()); EXPECT_EQ(StmtOf(lhs), assign); EXPECT_EQ(StmtOf(rhs), assign); @@ -110,7 +111,7 @@ ASSERT_NE(TypeOf(lhs), nullptr); ASSERT_NE(TypeOf(rhs), nullptr); - EXPECT_TRUE(TypeOf(lhs)->UnwrapAll()->Is<sem::F32>()); + EXPECT_TRUE(TypeOf(lhs)->UnwrapRef()->Is<sem::F32>()); EXPECT_TRUE(TypeOf(rhs)->Is<sem::F32>()); EXPECT_EQ(StmtOf(lhs), assign); EXPECT_EQ(StmtOf(rhs), assign); @@ -147,9 +148,9 @@ ASSERT_NE(TypeOf(lhs), nullptr); ASSERT_NE(TypeOf(rhs), nullptr); EXPECT_TRUE(TypeOf(stmt->condition())->Is<sem::Bool>()); - EXPECT_TRUE(TypeOf(else_lhs)->UnwrapAll()->Is<sem::F32>()); + EXPECT_TRUE(TypeOf(else_lhs)->UnwrapRef()->Is<sem::F32>()); EXPECT_TRUE(TypeOf(else_rhs)->Is<sem::F32>()); - EXPECT_TRUE(TypeOf(lhs)->UnwrapAll()->Is<sem::F32>()); + EXPECT_TRUE(TypeOf(lhs)->UnwrapRef()->Is<sem::F32>()); EXPECT_TRUE(TypeOf(rhs)->Is<sem::F32>()); EXPECT_EQ(StmtOf(lhs), assign); EXPECT_EQ(StmtOf(rhs), assign); @@ -180,9 +181,9 @@ ASSERT_NE(TypeOf(body_rhs), nullptr); ASSERT_NE(TypeOf(continuing_lhs), nullptr); ASSERT_NE(TypeOf(continuing_rhs), nullptr); - EXPECT_TRUE(TypeOf(body_lhs)->UnwrapAll()->Is<sem::F32>()); + EXPECT_TRUE(TypeOf(body_lhs)->UnwrapRef()->Is<sem::F32>()); EXPECT_TRUE(TypeOf(body_rhs)->Is<sem::F32>()); - EXPECT_TRUE(TypeOf(continuing_lhs)->UnwrapAll()->Is<sem::F32>()); + EXPECT_TRUE(TypeOf(continuing_lhs)->UnwrapRef()->Is<sem::F32>()); EXPECT_TRUE(TypeOf(continuing_rhs)->Is<sem::F32>()); EXPECT_EQ(BlockOf(body_lhs), body); EXPECT_EQ(BlockOf(body_rhs), body); @@ -224,7 +225,7 @@ ASSERT_NE(TypeOf(rhs), nullptr); EXPECT_TRUE(TypeOf(stmt->condition())->Is<sem::I32>()); - EXPECT_TRUE(TypeOf(lhs)->UnwrapAll()->Is<sem::F32>()); + EXPECT_TRUE(TypeOf(lhs)->UnwrapRef()->Is<sem::F32>()); EXPECT_TRUE(TypeOf(rhs)->Is<sem::F32>()); EXPECT_EQ(BlockOf(lhs), case_block); EXPECT_EQ(BlockOf(rhs), case_block); @@ -328,9 +329,9 @@ ASSERT_NE(TypeOf(foo_f32_init), nullptr); EXPECT_TRUE(TypeOf(foo_f32_init)->Is<sem::F32>()); ASSERT_NE(TypeOf(bar_i32_init), nullptr); - EXPECT_TRUE(TypeOf(bar_i32_init)->UnwrapAll()->Is<sem::I32>()); + EXPECT_TRUE(TypeOf(bar_i32_init)->UnwrapRef()->Is<sem::I32>()); ASSERT_NE(TypeOf(bar_f32_init), nullptr); - EXPECT_TRUE(TypeOf(bar_f32_init)->UnwrapAll()->Is<sem::F32>()); + EXPECT_TRUE(TypeOf(bar_f32_init)->UnwrapRef()->Is<sem::F32>()); EXPECT_EQ(StmtOf(foo_i32_init), foo_i32_decl); EXPECT_EQ(StmtOf(bar_i32_init), bar_i32_decl); EXPECT_EQ(StmtOf(foo_f32_init), foo_f32_decl); @@ -377,7 +378,7 @@ ASSERT_NE(TypeOf(fn_i32_init), nullptr); EXPECT_TRUE(TypeOf(fn_i32_init)->Is<sem::I32>()); ASSERT_NE(TypeOf(fn_f32_init), nullptr); - EXPECT_TRUE(TypeOf(fn_f32_init)->UnwrapAll()->Is<sem::F32>()); + EXPECT_TRUE(TypeOf(fn_f32_init)->UnwrapRef()->Is<sem::F32>()); EXPECT_EQ(StmtOf(fn_i32_init), fn_i32_decl); EXPECT_EQ(StmtOf(mod_init), nullptr); EXPECT_EQ(StmtOf(fn_f32_init), fn_f32_decl); @@ -397,10 +398,10 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); ASSERT_NE(TypeOf(acc), nullptr); - ASSERT_TRUE(TypeOf(acc)->Is<sem::Pointer>()); + ASSERT_TRUE(TypeOf(acc)->Is<sem::Reference>()); - auto* ptr = TypeOf(acc)->As<sem::Pointer>(); - EXPECT_TRUE(ptr->StoreType()->Is<sem::F32>()); + auto* ref = TypeOf(acc)->As<sem::Reference>(); + EXPECT_TRUE(ref->StoreType()->Is<sem::F32>()); } TEST_F(ResolverTest, Expr_ArrayAccessor_Alias_Array) { @@ -415,10 +416,10 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); ASSERT_NE(TypeOf(acc), nullptr); - ASSERT_TRUE(TypeOf(acc)->Is<sem::Pointer>()); + ASSERT_TRUE(TypeOf(acc)->Is<sem::Reference>()); - auto* ptr = TypeOf(acc)->As<sem::Pointer>(); - EXPECT_TRUE(ptr->StoreType()->Is<sem::F32>()); + auto* ref = TypeOf(acc)->As<sem::Reference>(); + EXPECT_TRUE(ref->StoreType()->Is<sem::F32>()); } TEST_F(ResolverTest, Expr_ArrayAccessor_Array_Constant) { @@ -442,11 +443,11 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); ASSERT_NE(TypeOf(acc), nullptr); - ASSERT_TRUE(TypeOf(acc)->Is<sem::Pointer>()); + ASSERT_TRUE(TypeOf(acc)->Is<sem::Reference>()); - auto* ptr = TypeOf(acc)->As<sem::Pointer>(); - ASSERT_TRUE(ptr->StoreType()->Is<sem::Vector>()); - EXPECT_EQ(ptr->StoreType()->As<sem::Vector>()->size(), 3u); + auto* ref = TypeOf(acc)->As<sem::Reference>(); + ASSERT_TRUE(ref->StoreType()->Is<sem::Vector>()); + EXPECT_EQ(ref->StoreType()->As<sem::Vector>()->size(), 3u); } TEST_F(ResolverTest, Expr_ArrayAccessor_Matrix_BothDimensions) { @@ -458,10 +459,10 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); ASSERT_NE(TypeOf(acc), nullptr); - ASSERT_TRUE(TypeOf(acc)->Is<sem::Pointer>()); + ASSERT_TRUE(TypeOf(acc)->Is<sem::Reference>()); - auto* ptr = TypeOf(acc)->As<sem::Pointer>(); - EXPECT_TRUE(ptr->StoreType()->Is<sem::F32>()); + auto* ref = TypeOf(acc)->As<sem::Reference>(); + EXPECT_TRUE(ref->StoreType()->Is<sem::F32>()); } TEST_F(ResolverTest, Expr_ArrayAccessor_Vector) { @@ -473,10 +474,10 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); ASSERT_NE(TypeOf(acc), nullptr); - ASSERT_TRUE(TypeOf(acc)->Is<sem::Pointer>()); + ASSERT_TRUE(TypeOf(acc)->Is<sem::Reference>()); - auto* ptr = TypeOf(acc)->As<sem::Pointer>(); - EXPECT_TRUE(ptr->StoreType()->Is<sem::F32>()); + auto* ref = TypeOf(acc)->As<sem::Reference>(); + EXPECT_TRUE(ref->StoreType()->Is<sem::F32>()); } TEST_F(ResolverTest, Expr_Bitcast) { @@ -519,7 +520,10 @@ TEST_F(ResolverTest, Expr_Call_WithParams) { ast::VariableList params; - Func("my_func", params, ty.void_(), {}, ast::DecorationList{}); + Func("my_func", params, ty.f32(), + { + Return(1.2f), + }); auto* param = Expr(2.4f); @@ -609,8 +613,8 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); ASSERT_NE(TypeOf(ident), nullptr); - EXPECT_TRUE(TypeOf(ident)->Is<sem::Pointer>()); - EXPECT_TRUE(TypeOf(ident)->As<sem::Pointer>()->StoreType()->Is<sem::F32>()); + ASSERT_TRUE(TypeOf(ident)->Is<sem::Reference>()); + EXPECT_TRUE(TypeOf(ident)->UnwrapRef()->Is<sem::F32>()); EXPECT_TRUE(CheckVarUsers(my_var, {ident})); ASSERT_NE(VarOf(ident), nullptr); EXPECT_EQ(VarOf(ident)->Declaration(), my_var); @@ -674,14 +678,12 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); ASSERT_NE(TypeOf(my_var_a), nullptr); - EXPECT_TRUE(TypeOf(my_var_a)->Is<sem::Pointer>()); - EXPECT_TRUE( - TypeOf(my_var_a)->As<sem::Pointer>()->StoreType()->Is<sem::F32>()); + ASSERT_TRUE(TypeOf(my_var_a)->Is<sem::Reference>()); + EXPECT_TRUE(TypeOf(my_var_a)->UnwrapRef()->Is<sem::F32>()); EXPECT_EQ(StmtOf(my_var_a), assign); ASSERT_NE(TypeOf(my_var_b), nullptr); - EXPECT_TRUE(TypeOf(my_var_b)->Is<sem::Pointer>()); - EXPECT_TRUE( - TypeOf(my_var_b)->As<sem::Pointer>()->StoreType()->Is<sem::F32>()); + ASSERT_TRUE(TypeOf(my_var_b)->Is<sem::Reference>()); + EXPECT_TRUE(TypeOf(my_var_b)->UnwrapRef()->Is<sem::F32>()); EXPECT_EQ(StmtOf(my_var_b), assign); EXPECT_TRUE(CheckVarUsers(var, {my_var_a, my_var_b})); ASSERT_NE(VarOf(my_var_a), nullptr); @@ -691,29 +693,30 @@ } TEST_F(ResolverTest, Expr_Identifier_Function_Ptr) { - auto* my_var_a = Expr("my_var"); - auto* my_var_b = Expr("my_var"); - auto* assign = Assign(my_var_a, my_var_b); - + auto* v = Expr("v"); + auto* p = Expr("p"); + auto* v_decl = Decl(Var("v", ty.f32())); + auto* p_decl = Decl( + Const("p", ty.pointer<f32>(ast::StorageClass::kFunction), AddressOf(v))); + auto* assign = Assign(Deref(p), 1.23f); Func("my_func", ast::VariableList{}, ty.void_(), { - Decl(Var("my_var", ty.pointer<f32>(ast::StorageClass::kFunction))), + v_decl, + p_decl, assign, }, ast::DecorationList{}); EXPECT_TRUE(r()->Resolve()) << r()->error(); - ASSERT_NE(TypeOf(my_var_a), nullptr); - EXPECT_TRUE(TypeOf(my_var_a)->Is<sem::Pointer>()); - EXPECT_TRUE( - TypeOf(my_var_a)->As<sem::Pointer>()->StoreType()->Is<sem::F32>()); - EXPECT_EQ(StmtOf(my_var_a), assign); - ASSERT_NE(TypeOf(my_var_b), nullptr); - EXPECT_TRUE(TypeOf(my_var_b)->Is<sem::Pointer>()); - EXPECT_TRUE( - TypeOf(my_var_b)->As<sem::Pointer>()->StoreType()->Is<sem::F32>()); - EXPECT_EQ(StmtOf(my_var_b), assign); + ASSERT_NE(TypeOf(v), nullptr); + ASSERT_TRUE(TypeOf(v)->Is<sem::Reference>()); + EXPECT_TRUE(TypeOf(v)->UnwrapRef()->Is<sem::F32>()); + EXPECT_EQ(StmtOf(v), p_decl); + ASSERT_NE(TypeOf(p), nullptr); + ASSERT_TRUE(TypeOf(p)->Is<sem::Pointer>()); + EXPECT_TRUE(TypeOf(p)->UnwrapPtr()->Is<sem::F32>()); + EXPECT_EQ(StmtOf(p), assign); } TEST_F(ResolverTest, Expr_Call_Function) { @@ -895,10 +898,10 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); ASSERT_NE(TypeOf(mem), nullptr); - ASSERT_TRUE(TypeOf(mem)->Is<sem::Pointer>()); + ASSERT_TRUE(TypeOf(mem)->Is<sem::Reference>()); - auto* ptr = TypeOf(mem)->As<sem::Pointer>(); - EXPECT_TRUE(ptr->StoreType()->Is<sem::F32>()); + auto* ref = TypeOf(mem)->As<sem::Reference>(); + EXPECT_TRUE(ref->StoreType()->Is<sem::F32>()); auto* sma = Sem().Get(mem)->As<sem::StructMemberAccess>(); ASSERT_NE(sma, nullptr); EXPECT_EQ(sma->Member()->Type(), ty.f32()); @@ -920,10 +923,10 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); ASSERT_NE(TypeOf(mem), nullptr); - ASSERT_TRUE(TypeOf(mem)->Is<sem::Pointer>()); + ASSERT_TRUE(TypeOf(mem)->Is<sem::Reference>()); - auto* ptr = TypeOf(mem)->As<sem::Pointer>(); - EXPECT_TRUE(ptr->StoreType()->Is<sem::F32>()); + auto* ref = TypeOf(mem)->As<sem::Reference>(); + EXPECT_TRUE(ref->StoreType()->Is<sem::F32>()); auto* sma = Sem().Get(mem)->As<sem::StructMemberAccess>(); ASSERT_NE(sma, nullptr); EXPECT_EQ(sma->Member()->Type(), ty.f32()); @@ -956,10 +959,10 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); ASSERT_NE(TypeOf(mem), nullptr); - ASSERT_TRUE(TypeOf(mem)->Is<sem::Pointer>()); + ASSERT_TRUE(TypeOf(mem)->Is<sem::Reference>()); - auto* ptr = TypeOf(mem)->As<sem::Pointer>(); - ASSERT_TRUE(ptr->StoreType()->Is<sem::F32>()); + auto* ref = TypeOf(mem)->As<sem::Reference>(); + ASSERT_TRUE(ref->StoreType()->Is<sem::F32>()); ASSERT_TRUE(Sem().Get(mem)->Is<sem::Swizzle>()); EXPECT_THAT(Sem().Get(mem)->As<sem::Swizzle>()->Indices(), ElementsAre(2)); }
diff --git a/src/resolver/type_constructor_validation_test.cc b/src/resolver/type_constructor_validation_test.cc index e5d06d0..94fc5ee 100644 --- a/src/resolver/type_constructor_validation_test.cc +++ b/src/resolver/type_constructor_validation_test.cc
@@ -13,6 +13,7 @@ // limitations under the License. #include "src/resolver/resolver_test_helper.h" +#include "src/sem/reference_type.h" namespace tint { namespace resolver { @@ -51,13 +52,19 @@ auto* a_ident = Expr("a"); auto* b_ident = Expr("b"); - WrapInFunction(Decl(a), Decl(b), Assign(a_ident, "a"), Assign(b_ident, "b")); + WrapInFunction(a, b, Assign(a_ident, "a"), Assign(b_ident, "b")); ASSERT_TRUE(r()->Resolve()) << r()->error(); - ASSERT_EQ(TypeOf(a_ident), - ty.pointer(ty.i32(), ast::StorageClass::kFunction)); - ASSERT_EQ(TypeOf(b_ident), - ty.pointer(ty.i32(), ast::StorageClass::kFunction)); + ASSERT_TRUE(TypeOf(a_ident)->Is<sem::Reference>()); + EXPECT_TRUE( + TypeOf(a_ident)->As<sem::Reference>()->StoreType()->Is<sem::I32>()); + EXPECT_EQ(TypeOf(a_ident)->As<sem::Reference>()->StorageClass(), + ast::StorageClass::kFunction); + ASSERT_TRUE(TypeOf(b_ident)->Is<sem::Reference>()); + EXPECT_TRUE( + TypeOf(b_ident)->As<sem::Reference>()->StoreType()->Is<sem::I32>()); + EXPECT_EQ(TypeOf(b_ident)->As<sem::Reference>()->StorageClass(), + ast::StorageClass::kFunction); } using InferTypeTest_FromConstructorExpression = ResolverTestWithParam<Params>; @@ -79,9 +86,8 @@ ASSERT_TRUE(r()->Resolve()) << r()->error(); auto* got = TypeOf(a_ident); - auto* expected = - ty.pointer(params.create_rhs_sem_type(ty), ast::StorageClass::kFunction) - .sem; + auto* expected = create<sem::Reference>(params.create_rhs_sem_type(ty), + ast::StorageClass::kFunction); ASSERT_EQ(got, expected) << "got: " << FriendlyName(got) << "\n" << "expected: " << FriendlyName(expected) << "\n"; } @@ -134,9 +140,8 @@ ASSERT_TRUE(r()->Resolve()) << r()->error(); auto* got = TypeOf(a_ident); - auto* expected = - ty.pointer(params.create_rhs_sem_type(ty), ast::StorageClass::kFunction) - .sem; + auto* expected = create<sem::Reference>(params.create_rhs_sem_type(ty), + ast::StorageClass::kFunction); ASSERT_EQ(got, expected) << "got: " << FriendlyName(got) << "\n" << "expected: " << FriendlyName(expected) << "\n"; } @@ -184,9 +189,8 @@ ASSERT_TRUE(r()->Resolve()) << r()->error(); auto* got = TypeOf(a_ident); - auto* expected = - ty.pointer(params.create_rhs_sem_type(ty), ast::StorageClass::kFunction) - .sem; + auto* expected = create<sem::Reference>(params.create_rhs_sem_type(ty), + ast::StorageClass::kFunction); ASSERT_EQ(got, expected) << "got: " << FriendlyName(got) << "\n" << "expected: " << FriendlyName(expected) << "\n"; }
diff --git a/src/resolver/type_validation_test.cc b/src/resolver/type_validation_test.cc index beec7d5..de11bb9 100644 --- a/src/resolver/type_validation_test.cc +++ b/src/resolver/type_validation_test.cc
@@ -49,27 +49,6 @@ ASSERT_NE(TypeOf(rhs), nullptr); } -TEST_F(ResolverTypeValidationTest, FunctionConstantNoConstructor_Fail) { - // { - // let a :i32; - // } - auto* var = Const(Source{{12, 34}}, "a", ty.i32(), nullptr); - WrapInFunction(var); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error: let declarations must have initializers"); -} - -TEST_F(ResolverTypeValidationTest, GlobalConstantNoConstructor_Fail) { - // let a :i32; - GlobalConst(Source{{12, 34}}, "a", ty.i32(), nullptr); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error: let declarations must have initializers"); -} - TEST_F(ResolverTypeValidationTest, GlobalConstantNoConstructor_Pass) { // [[override(0)]] let a :i32; GlobalConst(Source{{12, 34}}, "a", ty.i32(), nullptr, @@ -117,19 +96,6 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); } -TEST_F(ResolverTypeValidationTest, GlobalVariableNotUnique_Fail) { - // var global_var : f32 = 0.1; - // var global_var : i32 = 0; - Global("global_var", ty.f32(), ast::StorageClass::kPrivate, Expr(0.1f)); - - Global(Source{{12, 34}}, "global_var", ty.i32(), ast::StorageClass::kPrivate, - Expr(0)); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error v-0011: redeclared global identifier 'global_var'"); -} - TEST_F(ResolverTypeValidationTest, GlobalVariableFunctionVariableNotUnique_Pass) { // fn my_func() { @@ -146,48 +112,6 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); } -TEST_F(ResolverTypeValidationTest, - GlobalVariableFunctionVariableNotUnique_Fail) { - // var a: f32 = 2.1; - // fn my_func() { - // var a: f32 = 2.0; - // return 0; - // } - - Global("a", ty.f32(), ast::StorageClass::kPrivate, Expr(2.1f)); - - auto* var = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(2.0f)); - - Func("my_func", ast::VariableList{}, ty.void_(), - ast::StatementList{ - Decl(Source{{12, 34}}, var), - }, - ast::DecorationList{}); - - EXPECT_FALSE(r()->Resolve()) << r()->error(); - EXPECT_EQ(r()->error(), "12:34 error v-0013: redeclared identifier 'a'"); -} - -TEST_F(ResolverTypeValidationTest, RedeclaredIdentifier_Fail) { - // fn my_func()() { - // var a :i32 = 2; - // var a :f21 = 2.0; - // } - auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2)); - - auto* var_a_float = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(0.1f)); - - Func("my_func", ast::VariableList{}, ty.void_(), - ast::StatementList{ - Decl(var), - Decl(Source{{12, 34}}, var_a_float), - }, - ast::DecorationList{}); - - EXPECT_FALSE(r()->Resolve()) << r()->error(); - EXPECT_EQ(r()->error(), "12:34 error v-0014: redeclared identifier 'a'"); -} - TEST_F(ResolverTypeValidationTest, RedeclaredIdentifierInnerScope_Pass) { // { // if (true) { var a : f32 = 2.0; } @@ -209,31 +133,6 @@ EXPECT_TRUE(r()->Resolve()); } -TEST_F(ResolverTypeValidationTest, - DISABLED_RedeclaredIdentifierInnerScope_False) { - // TODO(sarahM0): remove DISABLED after implementing ValidateIfStatement - // and it should just work - // { - // var a : f32 = 3.14; - // if (true) { var a : f32 = 2.0; } - // } - auto* var_a_float = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(3.1f)); - - auto* var = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(2.0f)); - - auto* cond = Expr(true); - auto* body = Block(Decl(Source{{12, 34}}, var)); - - auto* outer_body = - Block(Decl(var_a_float), - create<ast::IfStatement>(cond, body, ast::ElseStatementList{})); - - WrapInFunction(outer_body); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), "12:34 error v-0014: redeclared identifier 'a'"); -} - TEST_F(ResolverTypeValidationTest, RedeclaredIdentifierInnerScopeBlock_Pass) { // { // { var a : f32; } @@ -250,23 +149,6 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); } -TEST_F(ResolverTypeValidationTest, RedeclaredIdentifierInnerScopeBlock_Fail) { - // { - // var a : f32; - // { var a : f32; } - // } - auto* var_inner = Var("a", ty.f32(), ast::StorageClass::kNone); - auto* inner = Block(Decl(Source{{12, 34}}, var_inner)); - - auto* var_outer = Var("a", ty.f32(), ast::StorageClass::kNone); - auto* outer_body = Block(Decl(var_outer), inner); - - WrapInFunction(outer_body); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), "12:34 error v-0014: redeclared identifier 'a'"); -} - TEST_F(ResolverTypeValidationTest, RedeclaredIdentifierDifferentFunctions_Pass) { // func0 { var a : f32 = 2.0; return; } @@ -519,7 +401,7 @@ EXPECT_TRUE(r()->Resolve()) << r()->error(); - auto* got = TypeOf(expr)->UnwrapPtr(); + auto* got = TypeOf(expr)->UnwrapRef(); auto* expected = params.create_sem_type(ty); EXPECT_EQ(got, expected) << "got: " << FriendlyName(got) << "\n"
diff --git a/src/resolver/validation_test.cc b/src/resolver/validation_test.cc index 9440f1f..ed4d089 100644 --- a/src/resolver/validation_test.cc +++ b/src/resolver/validation_test.cc
@@ -160,34 +160,6 @@ "12:34 error: else statement condition must be bool, got f32"); } -TEST_F(ResolverValidationTest, - Stmt_VariableDecl_MismatchedTypeScalarConstructor) { - u32 unsigned_value = 2u; // Type does not match variable type - auto* decl = Decl(Var(Source{{3, 3}}, "my_var", ty.i32(), - ast::StorageClass::kNone, Expr(unsigned_value))); - WrapInFunction(decl); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - R"(3:3 error: variable of type 'i32' cannot be initialized with a value of type 'u32')"); -} - -TEST_F(ResolverValidationTest, - Stmt_VariableDecl_MismatchedTypeScalarConstructor_Alias) { - auto* my_int = ty.alias("MyInt", ty.i32()); - AST().AddConstructedType(my_int); - u32 unsigned_value = 2u; // Type does not match variable type - auto* decl = Decl(Var(Source{{3, 3}}, "my_var", my_int, - ast::StorageClass::kNone, Expr(unsigned_value))); - WrapInFunction(decl); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - R"(3:3 error: variable of type 'MyInt' cannot be initialized with a value of type 'u32')"); -} - TEST_F(ResolverValidationTest, Expr_Error_Unknown) { auto* e = create<FakeExpr>(Source{Source::Location{2, 30}}); WrapInFunction(e);
diff --git a/src/resolver/var_let_test.cc b/src/resolver/var_let_test.cc new file mode 100644 index 0000000..bf37ace --- /dev/null +++ b/src/resolver/var_let_test.cc
@@ -0,0 +1,133 @@ +// 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 "src/resolver/resolver_test_helper.h" +#include "src/sem/reference_type.h" + +#include "gmock/gmock.h" + +namespace tint { +namespace resolver { +namespace { + +struct ResolverVarLetTest : public resolver::TestHelper, + public testing::Test {}; + +TEST_F(ResolverVarLetTest, TypeOfVar) { + // struct S { i : i32; } + // alias A = S; + // fn F(){ + // var i : i32; + // var u : u32; + // var f : f32; + // var b : bool; + // var s : S; + // var a : A; + // } + + auto* S = Structure("S", {Member("i", ty.i32())}); + auto* A = ty.alias("A", S); + AST().AddConstructedType(A); + + auto* i = Var("i", ty.i32(), ast::StorageClass::kNone); + auto* u = Var("u", ty.u32(), ast::StorageClass::kNone); + auto* f = Var("f", ty.f32(), ast::StorageClass::kNone); + auto* b = Var("b", ty.bool_(), ast::StorageClass::kNone); + auto* s = Var("s", S, ast::StorageClass::kNone); + auto* a = Var("a", A, ast::StorageClass::kNone); + + Func("F", {}, ty.void_(), + { + Decl(i), + Decl(u), + Decl(f), + Decl(b), + Decl(s), + Decl(a), + }); + + EXPECT_TRUE(r()->Resolve()) << r()->error(); + + // `var` declarations are always of reference type + ASSERT_TRUE(TypeOf(i)->Is<sem::Reference>()); + ASSERT_TRUE(TypeOf(u)->Is<sem::Reference>()); + ASSERT_TRUE(TypeOf(f)->Is<sem::Reference>()); + ASSERT_TRUE(TypeOf(b)->Is<sem::Reference>()); + ASSERT_TRUE(TypeOf(s)->Is<sem::Reference>()); + ASSERT_TRUE(TypeOf(a)->Is<sem::Reference>()); + + EXPECT_TRUE(TypeOf(i)->As<sem::Reference>()->StoreType()->Is<sem::I32>()); + EXPECT_TRUE(TypeOf(u)->As<sem::Reference>()->StoreType()->Is<sem::U32>()); + EXPECT_TRUE(TypeOf(f)->As<sem::Reference>()->StoreType()->Is<sem::F32>()); + EXPECT_TRUE(TypeOf(b)->As<sem::Reference>()->StoreType()->Is<sem::Bool>()); + EXPECT_TRUE(TypeOf(s)->As<sem::Reference>()->StoreType()->Is<sem::Struct>()); + EXPECT_TRUE(TypeOf(a)->As<sem::Reference>()->StoreType()->Is<sem::Struct>()); +} + +TEST_F(ResolverVarLetTest, TypeOfLet) { + // struct S { i : i32; } + // fn F(){ + // var v : i32; + // let i : i32 = 1; + // let u : u32 = 1u; + // let f : f32 = 1.; + // let b : bool = true; + // let s : S = S(1); + // let a : A = A(1); + // let p : pointer<function, i32> = &V; + // } + + auto* S = Structure("S", {Member("i", ty.i32())}); + auto* A = ty.alias("A", S); + AST().AddConstructedType(A); + + auto* v = Var("v", ty.i32(), ast::StorageClass::kNone); + auto* i = Const("i", ty.i32(), Expr(1)); + auto* u = Const("u", ty.u32(), Expr(1u)); + auto* f = Const("f", ty.f32(), Expr(1.f)); + auto* b = Const("b", ty.bool_(), Expr(true)); + auto* s = Const("s", S, Construct(S, Expr(1))); + auto* a = Const("a", A, Construct(A, Expr(1))); + auto* p = + Const("p", ty.pointer<i32>(ast::StorageClass::kFunction), AddressOf(v)); + + Func("F", {}, ty.void_(), + { + Decl(v), + Decl(i), + Decl(u), + Decl(f), + Decl(b), + Decl(s), + Decl(a), + Decl(p), + }); + + EXPECT_TRUE(r()->Resolve()) << r()->error(); + + // `let` declarations are always of the storage type + EXPECT_TRUE(TypeOf(i)->Is<sem::I32>()); + EXPECT_TRUE(TypeOf(u)->Is<sem::U32>()); + EXPECT_TRUE(TypeOf(f)->Is<sem::F32>()); + EXPECT_TRUE(TypeOf(b)->Is<sem::Bool>()); + EXPECT_TRUE(TypeOf(s)->Is<sem::Struct>()); + EXPECT_TRUE(TypeOf(a)->Is<sem::Struct>()); + ASSERT_TRUE(TypeOf(p)->Is<sem::Pointer>()); + EXPECT_TRUE(TypeOf(p)->As<sem::Pointer>()->StoreType()->Is<sem::I32>()); +} + +} // namespace +} // namespace resolver +} // namespace tint
diff --git a/src/resolver/var_let_validation_test.cc b/src/resolver/var_let_validation_test.cc new file mode 100644 index 0000000..23c42e9 --- /dev/null +++ b/src/resolver/var_let_validation_test.cc
@@ -0,0 +1,220 @@ +// 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 "src/resolver/resolver_test_helper.h" + +#include "gmock/gmock.h" + +namespace tint { +namespace resolver { +namespace { + +struct ResolverVarLetValidationTest : public resolver::TestHelper, + public testing::Test {}; + +TEST_F(ResolverVarLetValidationTest, LetNoInitializer) { + // let a : i32; + WrapInFunction(Const(Source{{12, 34}}, "a", ty.i32(), nullptr)); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: let declarations must have initializers"); +} + +TEST_F(ResolverVarLetValidationTest, GlobalLetNoInitializer) { + // let a : i32; + GlobalConst(Source{{12, 34}}, "a", ty.i32(), nullptr); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: let declarations must have initializers"); +} + +TEST_F(ResolverVarLetValidationTest, VarConstructorNotStorable) { + // var i : i32; + // var p : pointer<function, i32> = &v; + auto* i = Var("i", ty.i32(), ast::StorageClass::kNone); + auto* p = Var("a", ty.i32(), ast::StorageClass::kNone, + AddressOf(Source{{12, 34}}, "i")); + WrapInFunction(i, p); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: 'ptr<function, i32>' is not storable for assignment"); +} + +TEST_F(ResolverVarLetValidationTest, LetConstructorWrongType) { + // var v : i32 = 2u + WrapInFunction(Const(Source{{3, 3}}, "v", ty.i32(), Expr(2u))); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ( + r()->error(), + R"(3:3 error: cannot initialize let of type 'i32' with value of type 'u32')"); +} + +TEST_F(ResolverVarLetValidationTest, VarConstructorWrongType) { + // var v : i32 = 2u + WrapInFunction( + Var(Source{{3, 3}}, "v", ty.i32(), ast::StorageClass::kNone, Expr(2u))); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ( + r()->error(), + R"(3:3 error: cannot initialize var of type 'i32' with value of type 'u32')"); +} + +TEST_F(ResolverVarLetValidationTest, LetConstructorWrongTypeViaAlias) { + auto* a = ty.alias("I32", ty.i32()); + AST().AddConstructedType(a); + WrapInFunction(Const(Source{{3, 3}}, "v", a, Expr(2u))); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ( + r()->error(), + R"(3:3 error: cannot initialize let of type 'I32' with value of type 'u32')"); +} + +TEST_F(ResolverVarLetValidationTest, VarConstructorWrongTypeViaAlias) { + auto* a = ty.alias("I32", ty.i32()); + AST().AddConstructedType(a); + WrapInFunction( + Var(Source{{3, 3}}, "v", a, ast::StorageClass::kNone, Expr(2u))); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ( + r()->error(), + R"(3:3 error: cannot initialize var of type 'I32' with value of type 'u32')"); +} + +TEST_F(ResolverVarLetValidationTest, LetOfPtrConstructedWithRef) { + // var a : f32; + // let b : ptr<function,f32> = a; + const auto priv = ast::StorageClass::kFunction; + auto* var_a = Var("a", ty.f32(), priv); + auto* var_b = + Const(Source{{12, 34}}, "b", ty.pointer<float>(priv), Expr("a"), {}); + WrapInFunction(var_a, var_b); + + ASSERT_FALSE(r()->Resolve()); + + EXPECT_EQ( + r()->error(), + R"(12:34 error: cannot initialize let of type 'ptr<function, f32>' with value of type 'f32')"); +} + +TEST_F(ResolverVarLetValidationTest, LocalVarRedeclared) { + // var v : f32; + // var v : i32; + auto* v1 = Var("v", ty.f32(), ast::StorageClass::kNone); + auto* v2 = Var(Source{{12, 34}}, "v", ty.i32(), ast::StorageClass::kNone); + WrapInFunction(v1, v2); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error v-0014: redeclared identifier 'v'"); +} + +TEST_F(ResolverVarLetValidationTest, LocalLetRedeclared) { + // let l : f32 = 1.; + // let l : i32 = 0; + auto* l1 = Const("l", ty.f32(), Expr(1.f)); + auto* l2 = Const(Source{{12, 34}}, "l", ty.i32(), Expr(0)); + WrapInFunction(l1, l2); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error v-0014: redeclared identifier 'l'"); +} + +TEST_F(ResolverVarLetValidationTest, GlobalVarRedeclared) { + // var v : f32; + // var v : i32; + Global("v", ty.f32(), ast::StorageClass::kPrivate); + Global(Source{{12, 34}}, "v", ty.i32(), ast::StorageClass::kPrivate); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error v-0011: redeclared global identifier 'v'"); +} + +TEST_F(ResolverVarLetValidationTest, GlobalLetRedeclared) { + // let l : f32 = 0.1; + // let l : i32 = 0; + GlobalConst("l", ty.f32(), Expr(0.1f)); + GlobalConst(Source{{12, 34}}, "l", ty.i32(), Expr(0)); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error v-0011: redeclared global identifier 'l'"); +} + +TEST_F(ResolverVarLetValidationTest, GlobalVarRedeclaredAsLocal) { + // var v : f32 = 2.1; + // fn my_func() { + // var v : f32 = 2.0; + // return 0; + // } + + Global("v", ty.f32(), ast::StorageClass::kPrivate, Expr(2.1f)); + + WrapInFunction(Var(Source{{12, 34}}, "v", ty.f32(), ast::StorageClass::kNone, + Expr(2.0f))); + + EXPECT_FALSE(r()->Resolve()) << r()->error(); + EXPECT_EQ(r()->error(), "12:34 error v-0013: redeclared identifier 'v'"); +} + +TEST_F(ResolverVarLetValidationTest, VarRedeclaredInInnerBlock) { + // { + // var v : f32; + // { var v : f32; } + // } + auto* var_outer = Var("v", ty.f32(), ast::StorageClass::kNone); + auto* var_inner = + Var(Source{{12, 34}}, "v", ty.f32(), ast::StorageClass::kNone); + auto* inner = Block(Decl(var_inner)); + auto* outer_body = Block(Decl(var_outer), inner); + + WrapInFunction(outer_body); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error v-0014: redeclared identifier 'v'"); +} + +TEST_F(ResolverVarLetValidationTest, VarRedeclaredInIfBlock) { + // { + // var v : f32 = 3.14; + // if (true) { var v : f32 = 2.0; } + // } + auto* var_a_float = Var("v", ty.f32(), ast::StorageClass::kNone, Expr(3.1f)); + + auto* var = Var(Source{{12, 34}}, "v", ty.f32(), ast::StorageClass::kNone, + Expr(2.0f)); + + auto* cond = Expr(true); + auto* body = Block(Decl(var)); + + auto* outer_body = + Block(Decl(var_a_float), + create<ast::IfStatement>(cond, body, ast::ElseStatementList{})); + + WrapInFunction(outer_body); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error v-0014: redeclared identifier 'v'"); +} + +} // namespace +} // namespace resolver +} // namespace tint
diff --git a/src/sem/function.cc b/src/sem/function.cc index 35c2e71..0c5e844 100644 --- a/src/sem/function.cc +++ b/src/sem/function.cc
@@ -139,7 +139,7 @@ VariableBindings ret; for (auto* var : ReferencedModuleVariables()) { - auto* unwrapped_type = var->Type()->UnwrapAccess(); + auto* unwrapped_type = var->Type()->UnwrapRef(); auto* storage_texture = unwrapped_type->As<sem::StorageTexture>(); if (storage_texture == nullptr) { continue; @@ -156,7 +156,7 @@ VariableBindings ret; for (auto* var : ReferencedModuleVariables()) { - auto* unwrapped_type = var->Type()->UnwrapAccess(); + auto* unwrapped_type = var->Type()->UnwrapRef(); auto* storage_texture = unwrapped_type->As<sem::DepthTexture>(); if (storage_texture == nullptr) { continue; @@ -174,7 +174,7 @@ VariableBindings ret; for (auto* var : ReferencedModuleVariables()) { - auto* unwrapped_type = var->Type()->UnwrapAccess(); + auto* unwrapped_type = var->Type()->UnwrapRef(); auto* external_texture = unwrapped_type->As<sem::ExternalTexture>(); if (external_texture == nullptr) { continue; @@ -201,7 +201,7 @@ VariableBindings ret; for (auto* var : ReferencedModuleVariables()) { - auto* unwrapped_type = var->Type()->UnwrapAccess(); + auto* unwrapped_type = var->Type()->UnwrapRef(); auto* sampler = unwrapped_type->As<sem::Sampler>(); if (sampler == nullptr || sampler->kind() != kind) { continue; @@ -219,7 +219,7 @@ VariableBindings ret; for (auto* var : ReferencedModuleVariables()) { - auto* unwrapped_type = var->Type()->UnwrapAccess(); + auto* unwrapped_type = var->Type()->UnwrapRef(); auto* texture = unwrapped_type->As<sem::Texture>(); if (texture == nullptr) { continue;
diff --git a/src/sem/type.cc b/src/sem/type.cc index 1f7ffca..00f9e10 100644 --- a/src/sem/type.cc +++ b/src/sem/type.cc
@@ -19,6 +19,7 @@ #include "src/sem/i32_type.h" #include "src/sem/matrix_type.h" #include "src/sem/pointer_type.h" +#include "src/sem/reference_type.h" #include "src/sem/sampler_type.h" #include "src/sem/texture_type.h" #include "src/sem/u32_type.h" @@ -43,21 +44,17 @@ return type; } -const Type* Type::UnwrapAccess() const { - // TODO(amaiorano): Delete this function +const Type* Type::UnwrapRef() const { auto* type = this; + if (auto* ref = type->As<sem::Reference>()) { + type = ref->StoreType(); + } return type; } -const Type* Type::UnwrapAll() const { +const Type* Type::UnwrapAccess() const { + // TODO(amaiorano): Delete this function auto* type = this; - while (true) { - if (auto* ptr = type->As<sem::Pointer>()) { - type = ptr->StoreType(); - } else { - break; - } - } return type; }
diff --git a/src/sem/type.h b/src/sem/type.h index 76a64e1..11dd080 100644 --- a/src/sem/type.h +++ b/src/sem/type.h
@@ -49,15 +49,13 @@ /// otherwise const Type* UnwrapPtr() const; + /// @returns the inner type if this is a reference, `this` otherwise + const Type* UnwrapRef() const; + /// @returns the inner most type if this is an access control, `this` /// otherwise const Type* UnwrapAccess() const; - /// Returns the type found after removing all layers of access control and - /// pointer - /// @returns the unwrapped type - const Type* UnwrapAll() const; - /// @returns true if this type is a scalar bool is_scalar() const; /// @returns true if this type is a numeric scalar
diff --git a/src/transform/binding_remapper.cc b/src/transform/binding_remapper.cc index 8a1c42c..f89bfd9 100644 --- a/src/transform/binding_remapper.cc +++ b/src/transform/binding_remapper.cc
@@ -113,7 +113,7 @@ auto ac_it = remappings->access_controls.find(from); if (ac_it != remappings->access_controls.end()) { ast::AccessControl::Access ac = ac_it->second; - auto* ty = in->Sem().Get(var)->Type(); + auto* ty = in->Sem().Get(var)->Type()->UnwrapRef(); ast::Type* inner_ty = CreateASTTypeFor(&ctx, ty); auto* new_ty = ctx.dst->create<ast::AccessControl>(ac, inner_ty); auto* new_var = ctx.dst->create<ast::Variable>(
diff --git a/src/transform/bound_array_accessors.cc b/src/transform/bound_array_accessors.cc index 82c221b..19a9643 100644 --- a/src/transform/bound_array_accessors.cc +++ b/src/transform/bound_array_accessors.cc
@@ -41,7 +41,7 @@ CloneContext* ctx) { auto& diags = ctx->dst->Diagnostics(); - auto* ret_type = ctx->src->Sem().Get(expr->array())->Type()->UnwrapAll(); + auto* ret_type = ctx->src->Sem().Get(expr->array())->Type()->UnwrapRef(); if (!ret_type->Is<sem::Array>() && !ret_type->Is<sem::Matrix>() && !ret_type->Is<sem::Vector>()) { return nullptr;
diff --git a/src/transform/bound_array_accessors_test.cc b/src/transform/bound_array_accessors_test.cc index 00414f1..b9120d4 100644 --- a/src/transform/bound_array_accessors_test.cc +++ b/src/transform/bound_array_accessors_test.cc
@@ -29,7 +29,7 @@ let c : u32 = 1u; fn f() { - let b : ptr<private, f32> = a[c]; + let b : f32 = a[c]; } )"; @@ -39,7 +39,7 @@ let c : u32 = 1u; fn f() { - let b : ptr<private, f32> = a[min(u32(c), 2u)]; + let b : f32 = a[min(u32(c), 2u)]; } )";
diff --git a/src/transform/calculate_array_length.cc b/src/transform/calculate_array_length.cc index 96efb9d..e6becb4 100644 --- a/src/transform/calculate_array_length.cc +++ b/src/transform/calculate_array_length.cc
@@ -142,7 +142,7 @@ auto* storage_buffer_expr = accessor->structure(); auto* storage_buffer_sem = sem.Get(storage_buffer_expr); auto* storage_buffer_type = - storage_buffer_sem->Type()->UnwrapAll()->As<sem::Struct>(); + storage_buffer_sem->Type()->UnwrapRef()->As<sem::Struct>(); // Generate BufferSizeIntrinsic for this storage type if we haven't // already
diff --git a/src/transform/canonicalize_entry_point_io.cc b/src/transform/canonicalize_entry_point_io.cc index 8da983f..bb1cb71 100644 --- a/src/transform/canonicalize_entry_point_io.cc +++ b/src/transform/canonicalize_entry_point_io.cc
@@ -115,7 +115,7 @@ // Pull out all struct members and build initializer list. std::vector<Symbol> member_names; for (auto* member : str->Members()) { - if (member->Type()->UnwrapAll()->Is<sem::Struct>()) { + if (member->Type()->Is<sem::Struct>()) { TINT_ICE(ctx.dst->Diagnostics()) << "nested pipeline IO struct"; } @@ -205,7 +205,7 @@ if (auto* str = ret_type->As<sem::Struct>()) { // Rebuild struct with only the entry point IO attributes. for (auto* member : str->Members()) { - if (member->Type()->UnwrapAll()->Is<sem::Struct>()) { + if (member->Type()->Is<sem::Struct>()) { TINT_ICE(ctx.dst->Diagnostics()) << "nested pipeline IO struct"; }
diff --git a/src/transform/decompose_storage_access.cc b/src/transform/decompose_storage_access.cc index ab256a2..46495a1 100644 --- a/src/transform/decompose_storage_access.cc +++ b/src/transform/decompose_storage_access.cc
@@ -29,6 +29,7 @@ #include "src/sem/array.h" #include "src/sem/call.h" #include "src/sem/member_accessor_expression.h" +#include "src/sem/reference_type.h" #include "src/sem/struct.h" #include "src/sem/variable.h" #include "src/utils/get_or_create.h" @@ -56,7 +57,7 @@ explicit OffsetExpr(ast::Expression* e) : expr(e) {} ast::Expression* Build(CloneContext& ctx) override { - auto* type = ctx.src->Sem().Get(expr)->Type()->UnwrapAll(); + auto* type = ctx.src->Sem().Get(expr)->Type()->UnwrapRef(); auto* res = ctx.Clone(expr); if (!type->Is<sem::U32>()) { res = ctx.dst->Construct<ProgramBuilder::u32>(res); @@ -333,8 +334,8 @@ /// @returns the unwrapped, user-declared constructed type of ty. const ast::NamedType* ConstructedTypeOf(const sem::Type* ty) { while (true) { - if (auto* ptr = ty->As<sem::Pointer>()) { - ty = ptr->StoreType(); + if (auto* ref = ty->As<sem::Reference>()) { + ty = ref->StoreType(); continue; } if (auto* str = ty->As<sem::Struct>()) { @@ -466,14 +467,14 @@ for (auto* member : str->Members()) { auto* offset = ctx.dst->Add("offset", member->Offset()); Symbol load = LoadFunc(ctx, insert_after, buf_ty, - member->Type()->UnwrapAll(), var_user); + member->Type()->UnwrapRef(), var_user); values.emplace_back(ctx.dst->Call(load, "buffer", offset)); } } else if (auto* arr = el_ty->As<sem::Array>()) { for (uint32_t i = 0; i < arr->Count(); i++) { auto* offset = ctx.dst->Add("offset", arr->Stride() * i); Symbol load = LoadFunc(ctx, insert_after, buf_ty, - arr->ElemType()->UnwrapAll(), var_user); + arr->ElemType()->UnwrapRef(), var_user); values.emplace_back(ctx.dst->Call(load, "buffer", offset)); } } @@ -546,7 +547,7 @@ auto* access = ctx.dst->MemberAccessor( "value", ctx.Clone(member->Declaration()->symbol())); Symbol store = StoreFunc(ctx, insert_after, buf_ty, - member->Type()->UnwrapAll(), var_user); + member->Type()->UnwrapRef(), var_user); auto* call = ctx.dst->Call(store, "buffer", offset, access); body.emplace_back(ctx.dst->create<ast::CallStatement>(call)); } @@ -555,7 +556,7 @@ auto* offset = ctx.dst->Add("offset", arr->Stride() * i); auto* access = ctx.dst->IndexAccessor("value", ctx.dst->Expr(i)); Symbol store = StoreFunc(ctx, insert_after, buf_ty, - arr->ElemType()->UnwrapAll(), var_user); + arr->ElemType()->UnwrapRef(), var_user); auto* call = ctx.dst->Call(store, "buffer", offset, access); body.emplace_back(ctx.dst->create<ast::CallStatement>(call)); } @@ -661,7 +662,7 @@ state.AddAccess(ident, { var, ToOffset(0u), - var->Type()->UnwrapAll(), + var->Type()->UnwrapRef(), }); } } @@ -681,7 +682,7 @@ accessor, { access.var, Add(std::move(access.offset), std::move(offset)), - vec_ty->type()->UnwrapAll(), + vec_ty->type()->UnwrapRef(), }); } } @@ -694,7 +695,7 @@ { access.var, Add(std::move(access.offset), std::move(offset)), - member->Type()->UnwrapAll(), + member->Type()->UnwrapRef(), }); } } @@ -710,7 +711,7 @@ { access.var, Add(std::move(access.offset), std::move(offset)), - arr->ElemType()->UnwrapAll(), + arr->ElemType()->UnwrapRef(), }); continue; } @@ -720,7 +721,7 @@ { access.var, Add(std::move(access.offset), std::move(offset)), - vec_ty->type()->UnwrapAll(), + vec_ty->type()->UnwrapRef(), }); continue; } @@ -770,8 +771,8 @@ auto* buf = access.var->Declaration(); auto* offset = access.offset->Build(ctx); - auto* buf_ty = access.var->Type()->UnwrapPtr(); - auto* el_ty = access.type->UnwrapAll(); + auto* buf_ty = access.var->Type()->UnwrapRef(); + auto* el_ty = access.type->UnwrapRef(); auto* insert_after = ConstructedTypeOf(access.var->Type()); Symbol func = state.LoadFunc(ctx, insert_after, buf_ty, el_ty, access.var->As<sem::VariableUser>()); @@ -785,8 +786,8 @@ for (auto& store : state.stores) { auto* buf = store.target.var->Declaration(); auto* offset = store.target.offset->Build(ctx); - auto* buf_ty = store.target.var->Type()->UnwrapPtr(); - auto* el_ty = store.target.type->UnwrapAll(); + auto* buf_ty = store.target.var->Type()->UnwrapRef(); + auto* el_ty = store.target.type->UnwrapRef(); auto* value = store.assignment->rhs(); auto* insert_after = ConstructedTypeOf(store.target.var->Type()); Symbol func = state.StoreFunc(ctx, insert_after, buf_ty, el_ty,
diff --git a/src/transform/external_texture_transform.cc b/src/transform/external_texture_transform.cc index d4095f5..c8fb07f 100644 --- a/src/transform/external_texture_transform.cc +++ b/src/transform/external_texture_transform.cc
@@ -52,7 +52,10 @@ // if the first parameter is an external texture. if (auto* var = sem.Get(call_expr->params()[0])->As<sem::VariableUser>()) { - if (var->Variable()->Type()->Is<sem::ExternalTexture>()) { + if (var->Variable() + ->Type() + ->UnwrapRef() + ->Is<sem::ExternalTexture>()) { if (intrinsic->Type() == sem::IntrinsicType::kTextureLoad && call_expr->params().size() != 2) { TINT_ICE(ctx.dst->Diagnostics())
diff --git a/src/transform/transform.cc b/src/transform/transform.cc index d7ead87..f288451 100644 --- a/src/transform/transform.cc +++ b/src/transform/transform.cc
@@ -17,6 +17,7 @@ #include <algorithm> #include "src/program_builder.h" +#include "src/sem/reference_type.h" TINT_INSTANTIATE_TYPEINFO(tint::transform::Data); @@ -107,6 +108,9 @@ return ctx->dst->create<ast::TypeName>( ctx->Clone(s->Declaration()->name())); } + if (auto* s = ty->As<sem::Reference>()) { + return CreateASTTypeFor(ctx, s->StoreType()); + } TINT_UNREACHABLE(ctx->dst->Diagnostics()) << "Unhandled type: " << ty->TypeInfo().name; return nullptr;
diff --git a/src/writer/append_vector.cc b/src/writer/append_vector.cc index 9e64ac4..dcf179f 100644 --- a/src/writer/append_vector.cc +++ b/src/writer/append_vector.cc
@@ -41,7 +41,7 @@ uint32_t packed_size; const sem::Type* packed_el_sem_ty; auto* vector_sem = b->Sem().Get(vector); - auto* vector_ty = vector_sem->Type()->UnwrapPtr(); + auto* vector_ty = vector_sem->Type()->UnwrapRef(); if (auto* vec = vector_ty->As<sem::Vector>()) { packed_size = vec->size() + 1; packed_el_sem_ty = vec->type(); @@ -72,7 +72,7 @@ } else { packed.emplace_back(vector); } - if (packed_el_sem_ty != b->TypeOf(scalar)->UnwrapPtr()) { + if (packed_el_sem_ty != b->TypeOf(scalar)->UnwrapRef()) { // Cast scalar to the vector element type auto* scalar_cast = b->Construct(packed_el_ty, scalar); b->Sem().Add(scalar_cast, b->create<sem::Expression>(
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index 95a77f2..e894106 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc
@@ -317,8 +317,8 @@ return true; } - auto* lhs_type = TypeOf(expr->lhs())->UnwrapAll(); - auto* rhs_type = TypeOf(expr->rhs())->UnwrapAll(); + auto* lhs_type = TypeOf(expr->lhs())->UnwrapRef(); + auto* rhs_type = TypeOf(expr->rhs())->UnwrapRef(); // Multiplying by a matrix requires the use of `mul` in order to get the // type of multiply we desire. if (expr->op() == ast::BinaryOp::kMultiply && @@ -854,7 +854,7 @@ return false; } - auto* texture_type = TypeOf(texture)->UnwrapAll()->As<sem::Texture>(); + auto* texture_type = TypeOf(texture)->UnwrapRef()->As<sem::Texture>(); switch (intrinsic->Type()) { case sem::IntrinsicType::kTextureDimensions: @@ -1300,7 +1300,7 @@ bool GeneratorImpl::EmitTypeConstructor(std::ostream& pre, std::ostream& out, ast::TypeConstructorExpression* expr) { - auto* type = TypeOf(expr); + auto* type = TypeOf(expr)->UnwrapRef(); // If the type constructor is empty then we need to construct with the zero // value for all components. @@ -1699,7 +1699,7 @@ continue; // Global already emitted } - auto* type = var->Type()->UnwrapAccess(); + auto* type = var->Type()->UnwrapRef(); if (auto* strct = type->As<sem::Struct>()) { out << "ConstantBuffer<" << builder_.Symbols().NameFor(strct->Declaration()->name()) << "> " @@ -1748,8 +1748,9 @@ return false; } - if (!EmitType(out, var->Type(), ast::StorageClass::kStorage, - var->AccessControl(), "")) { + auto* type = var->Type()->UnwrapRef(); + if (!EmitType(out, type, ast::StorageClass::kStorage, var->AccessControl(), + "")) { return false; } @@ -1781,7 +1782,7 @@ auto* var = data.first; auto* deco = data.second; auto* sem = builder_.Sem().Get(var); - auto* type = sem->Type(); + auto* type = sem->Type()->UnwrapRef(); make_indent(out); if (!EmitType(out, type, sem->StorageClass(), sem->AccessControl(), @@ -1832,7 +1833,7 @@ auto* var = data.first; auto* deco = data.second; auto* sem = builder_.Sem().Get(var); - auto* type = sem->Type(); + auto* type = sem->Type()->UnwrapRef(); make_indent(out); if (!EmitType(out, type, sem->StorageClass(), sem->AccessControl(), @@ -1877,7 +1878,7 @@ for (auto* var : func_sem->ReferencedModuleVariables()) { auto* decl = var->Declaration(); - auto* unwrapped_type = var->Type()->UnwrapAll(); + auto* unwrapped_type = var->Type()->UnwrapRef(); if (!emitted_globals.emplace(decl->symbol()).second) { continue; // Global already emitted } @@ -1903,11 +1904,12 @@ } auto name = builder_.Symbols().NameFor(decl->symbol()); - if (!EmitType(out, var->Type(), var->StorageClass(), var->AccessControl(), + auto* type = var->Type()->UnwrapRef(); + if (!EmitType(out, type, var->StorageClass(), var->AccessControl(), name)) { return false; } - if (!var->Type()->Is<sem::Array>()) { + if (!type->Is<sem::Array>()) { out << " " << name; } @@ -2230,7 +2232,8 @@ if (var->constructor() != nullptr) { out << constructor_out.str(); } else { - if (!EmitZeroValue(out, builder_.Sem().Get(var)->Type())) { + auto* type = builder_.Sem().Get(var)->Type()->UnwrapRef(); + if (!EmitZeroValue(out, type)) { return false; } } @@ -2639,7 +2642,7 @@ make_indent(out); auto* sem = builder_.Sem().Get(var); - auto* type = sem->Type(); + auto* type = sem->Type()->UnwrapRef(); // TODO(dsinclair): Handle variable decorations if (!var->decorations().empty()) {
diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc index 08255ae..c4d25cf 100644 --- a/src/writer/msl/generator_impl.cc +++ b/src/writer/msl/generator_impl.cc
@@ -41,6 +41,7 @@ #include "src/sem/member_accessor_expression.h" #include "src/sem/multisampled_texture_type.h" #include "src/sem/pointer_type.h" +#include "src/sem/reference_type.h" #include "src/sem/sampled_texture_type.h" #include "src/sem/storage_texture_type.h" #include "src/sem/struct.h" @@ -177,7 +178,7 @@ bool GeneratorImpl::EmitBitcast(ast::BitcastExpression* expr) { out_ << "as_type<"; - if (!EmitType(TypeOf(expr), "")) { + if (!EmitType(TypeOf(expr)->UnwrapRef(), "")) { return false; } @@ -484,7 +485,7 @@ return false; } - auto* texture_type = TypeOf(texture)->UnwrapAll()->As<sem::Texture>(); + auto* texture_type = TypeOf(texture)->UnwrapRef()->As<sem::Texture>(); switch (intrinsic->Type()) { case sem::IntrinsicType::kTextureDimensions: { @@ -530,7 +531,7 @@ get_dim(dims[0]); out_ << ")"; } else { - EmitType(TypeOf(expr), ""); + EmitType(TypeOf(expr)->UnwrapRef(), ""); out_ << "("; for (size_t i = 0; i < dims.size(); i++) { if (i > 0) { @@ -880,7 +881,7 @@ } bool GeneratorImpl::EmitTypeConstructor(ast::TypeConstructorExpression* expr) { - auto* type = TypeOf(expr); + auto* type = TypeOf(expr)->UnwrapRef(); if (type->IsAnyOf<sem::Array, sem::Struct>()) { out_ << "{"; @@ -1016,7 +1017,7 @@ uint32_t loc = data.second; make_indent(); - if (!EmitType(program_->Sem().Get(var)->Type(), + if (!EmitType(program_->Sem().Get(var)->Type()->UnwrapRef(), program_->Symbols().NameFor(var->symbol()))) { return false; } @@ -1053,7 +1054,7 @@ auto* deco = data.second; make_indent(); - if (!EmitType(program_->Sem().Get(var)->Type(), + if (!EmitType(program_->Sem().Get(var)->Type()->UnwrapRef(), program_->Symbols().NameFor(var->symbol()))) { return false; } @@ -1267,7 +1268,7 @@ first = false; out_ << "thread "; - if (!EmitType(var->Type(), "")) { + if (!EmitType(var->Type()->UnwrapRef(), "")) { return false; } out_ << "& " << program_->Symbols().NameFor(var->Declaration()->symbol()); @@ -1282,7 +1283,7 @@ out_ << "constant "; // TODO(dsinclair): Can arrays be uniform? If so, fix this ... - if (!EmitType(var->Type(), "")) { + if (!EmitType(var->Type()->UnwrapRef(), "")) { return false; } out_ << "& " << program_->Symbols().NameFor(var->Declaration()->symbol()); @@ -1300,7 +1301,7 @@ } out_ << "device "; - if (!EmitType(var->Type(), "")) { + if (!EmitType(var->Type()->UnwrapRef(), "")) { return false; } out_ << "& " << program_->Symbols().NameFor(var->Declaration()->symbol()); @@ -1414,7 +1415,7 @@ } first = false; - auto* type = program_->Sem().Get(var)->Type(); + auto* type = program_->Sem().Get(var)->Type()->UnwrapRef(); if (!EmitType(type, "")) { return false; @@ -1462,7 +1463,7 @@ auto* builtin = data.second; - if (!EmitType(var->Type(), "")) { + if (!EmitType(var->Type()->UnwrapRef(), "")) { return false; } @@ -1497,7 +1498,7 @@ out_ << "constant "; // TODO(dsinclair): Can you have a uniform array? If so, this needs to be // updated to handle arrays property. - if (!EmitType(var->Type(), "")) { + if (!EmitType(var->Type()->UnwrapRef(), "")) { return false; } out_ << "& " << program_->Symbols().NameFor(var->Declaration()->symbol()) @@ -1522,7 +1523,7 @@ } out_ << "device "; - if (!EmitType(var->Type(), "")) { + if (!EmitType(var->Type()->UnwrapRef(), "")) { return false; } out_ << "& " << program_->Symbols().NameFor(var->Declaration()->symbol()) @@ -1659,7 +1660,8 @@ return false; } } else { - if (!EmitZeroValue(program_->Sem().Get(var)->Type())) { + auto* type = program_->Sem().Get(var)->Type()->UnwrapRef(); + if (!EmitZeroValue(type)) { return false; } } @@ -2186,13 +2188,14 @@ diagnostics_.add_error("Variable decorations are not handled yet"); return false; } - if (decl->is_const()) { - out_ << "const "; - } - if (!EmitType(var->Type(), program_->Symbols().NameFor(decl->symbol()))) { + auto* type = var->Type()->UnwrapRef(); + if (!EmitType(type, program_->Symbols().NameFor(decl->symbol()))) { return false; } - if (!var->Type()->Is<sem::Array>()) { + if (decl->is_const()) { + out_ << " const"; + } + if (!type->Is<sem::Array>()) { out_ << " " << program_->Symbols().NameFor(decl->symbol()); } @@ -2206,7 +2209,7 @@ var->StorageClass() == ast::StorageClass::kFunction || var->StorageClass() == ast::StorageClass::kNone || var->StorageClass() == ast::StorageClass::kOutput) { - if (!EmitZeroValue(var->Type())) { + if (!EmitZeroValue(type)) { return false; } } @@ -2231,7 +2234,7 @@ } out_ << "constant "; - auto* type = program_->Sem().Get(var)->Type(); + auto* type = program_->Sem().Get(var)->Type()->UnwrapRef(); if (!EmitType(type, program_->Symbols().NameFor(var->symbol()))) { return false; } @@ -2265,7 +2268,7 @@ // https://developer.apple.com/metal/Metal-Shading-Language-Specification.pdf // 2.2.3 Packed Vector Types auto num_els = vec->size(); - auto* el_ty = vec->type()->UnwrapAll(); + auto* el_ty = vec->type(); if (el_ty->IsAnyOf<sem::U32, sem::I32, sem::F32>()) { return SizeAndAlign{num_els * 4, 4}; } @@ -2276,7 +2279,7 @@ // 2.3 Matrix Data Types auto cols = mat->columns(); auto rows = mat->rows(); - auto* el_ty = mat->type()->UnwrapAll(); + auto* el_ty = mat->type(); if (el_ty->IsAnyOf<sem::U32, sem::I32, sem::F32>()) { static constexpr SizeAndAlign table[] = { /* float2x2 */ {16, 8},
diff --git a/src/writer/msl/generator_impl_function_test.cc b/src/writer/msl/generator_impl_function_test.cc index dafc9ca..83ffab0 100644 --- a/src/writer/msl/generator_impl_function_test.cc +++ b/src/writer/msl/generator_impl_function_test.cc
@@ -112,7 +112,7 @@ }; fragment tint_symbol_2 frag_main(tint_symbol_1 tint_symbol [[stage_in]]) { - const float foo = tint_symbol.foo; + float const foo = tint_symbol.foo; return {foo}; } @@ -146,7 +146,7 @@ }; fragment tint_symbol_2 frag_main(tint_symbol_1 tint_symbol [[stage_in]]) { - const float4 coord = tint_symbol.coord; + float4 const coord = tint_symbol.coord; return {coord.x}; } @@ -214,14 +214,14 @@ }; vertex tint_symbol vert_main() { - const Interface tint_symbol_1 = {0.5f, 0.25f, float4(0.0f)}; + Interface const tint_symbol_1 = {0.5f, 0.25f, float4(0.0f)}; return {tint_symbol_1.col1, tint_symbol_1.col2, tint_symbol_1.pos}; } fragment void frag_main(tint_symbol_3 tint_symbol_2 [[stage_in]]) { - const Interface colors = {tint_symbol_2.col1, tint_symbol_2.col2, tint_symbol_2.pos}; - const float r = colors.col1; - const float g = colors.col2; + Interface const colors = {tint_symbol_2.col1, tint_symbol_2.col2, tint_symbol_2.pos}; + float const r = colors.col1; + float const g = colors.col2; return; } @@ -283,12 +283,12 @@ } vertex tint_symbol vert_main1() { - const VertexOutput tint_symbol_1 = {foo(0.5f)}; + VertexOutput const tint_symbol_1 = {foo(0.5f)}; return {tint_symbol_1.pos}; } vertex tint_symbol_2 vert_main2() { - const VertexOutput tint_symbol_3 = {foo(0.25f)}; + VertexOutput const tint_symbol_3 = {foo(0.25f)}; return {tint_symbol_3.pos}; }
diff --git a/src/writer/msl/generator_impl_intrinsic_texture_test.cc b/src/writer/msl/generator_impl_intrinsic_texture_test.cc index 02d058d..901f100 100644 --- a/src/writer/msl/generator_impl_intrinsic_texture_test.cc +++ b/src/writer/msl/generator_impl_intrinsic_texture_test.cc
@@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include "src/ast/call_statement.h" #include "src/ast/intrinsic_texture_helper_test.h" #include "src/writer/msl/test_helper.h" @@ -259,7 +260,14 @@ auto* call = create<ast::CallExpression>(Expr(param.function), param.args(this)); - WrapInFunction(call); + + Func("main", ast::VariableList{}, ty.void_(), + ast::StatementList{ + create<ast::CallStatement>(call), + }, + ast::DecorationList{ + Stage(ast::PipelineStage::kFragment), + }); GeneratorImpl& gen = Build();
diff --git a/src/writer/msl/generator_impl_variable_decl_statement_test.cc b/src/writer/msl/generator_impl_variable_decl_statement_test.cc index 04e7e34..548a705 100644 --- a/src/writer/msl/generator_impl_variable_decl_statement_test.cc +++ b/src/writer/msl/generator_impl_variable_decl_statement_test.cc
@@ -48,7 +48,7 @@ gen.increment_indent(); ASSERT_TRUE(gen.EmitStatement(stmt)) << gen.error(); - EXPECT_EQ(gen.result(), " const float a = float(0.0f);\n"); + EXPECT_EQ(gen.result(), " float const a = float(0.0f);\n"); } TEST_F(MslGeneratorImplTest, Emit_VariableDeclStatement_Array) {
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 9c2cf36..22f4a6a 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc
@@ -28,9 +28,11 @@ #include "src/sem/intrinsic.h" #include "src/sem/member_accessor_expression.h" #include "src/sem/multisampled_texture_type.h" +#include "src/sem/reference_type.h" #include "src/sem/sampled_texture_type.h" #include "src/sem/struct.h" #include "src/sem/variable.h" +#include "src/utils/get_or_create.h" #include "src/writer/append_vector.h" namespace tint { @@ -357,7 +359,7 @@ return false; } - // If the thing we're assigning is a pointer then we must load it first. + // If the thing we're assigning is a reference then we must load it first. auto* type = TypeOf(assign->rhs()); rhs_id = GenerateLoadIfNeeded(type, rhs_id); @@ -598,7 +600,9 @@ return false; } auto* type = TypeOf(var->constructor()); - init_id = GenerateLoadIfNeeded(type, init_id); + if (type->Is<sem::Reference>()) { + init_id = GenerateLoadIfNeeded(type, init_id); + } } if (var->is_const()) { @@ -615,8 +619,7 @@ auto var_id = result.to_i(); auto sc = ast::StorageClass::kFunction; auto* type = builder_.Sem().Get(var)->Type(); - sem::Pointer pt(type, sc); - auto type_id = GenerateTypeIfNeeded(&pt); + auto type_id = GenerateTypeIfNeeded(type); if (type_id == 0) { return false; } @@ -627,7 +630,7 @@ // TODO(dsinclair) We could detect if the constructor is fully const and emit // an initializer value for the variable instead of doing the OpLoad. - auto null_id = GenerateConstantNullIfNeeded(type->UnwrapPtr()); + auto null_id = GenerateConstantNullIfNeeded(type->UnwrapRef()); if (null_id == 0) { return 0; } @@ -654,6 +657,7 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) { auto* sem = builder_.Sem().Get(var); + auto* type = sem->Type()->UnwrapRef(); uint32_t init_id = 0; if (var->has_constructor()) { @@ -679,16 +683,16 @@ } // SPIR-V requires specialization constants to have initializers. - if (sem->Type()->Is<sem::F32>()) { + if (type->Is<sem::F32>()) { ast::FloatLiteral l(ProgramID(), Source{}, 0.0f); init_id = GenerateLiteralIfNeeded(var, &l); - } else if (sem->Type()->Is<sem::U32>()) { + } else if (type->Is<sem::U32>()) { ast::UintLiteral l(ProgramID(), Source{}, 0); init_id = GenerateLiteralIfNeeded(var, &l); - } else if (sem->Type()->Is<sem::I32>()) { + } else if (type->Is<sem::I32>()) { ast::SintLiteral l(ProgramID(), Source{}, 0); init_id = GenerateLiteralIfNeeded(var, &l); - } else if (sem->Type()->Is<sem::Bool>()) { + } else if (type->Is<sem::Bool>()) { ast::BoolLiteral l(ProgramID(), Source{}, false); init_id = GenerateLiteralIfNeeded(var, &l); } else { @@ -715,8 +719,7 @@ ? ast::StorageClass::kPrivate : sem->StorageClass(); - sem::Pointer pt(sem->Type(), sc); - auto type_id = GenerateTypeIfNeeded(&pt); + auto type_id = GenerateTypeIfNeeded(sem->Type()); if (type_id == 0) { return false; } @@ -728,8 +731,6 @@ OperandList ops = {Operand::Int(type_id), result, Operand::Int(ConvertStorageClass(sc))}; - auto* type = sem->Type(); - if (var->has_constructor()) { ops.push_back(Operand::Int(init_id)); } else if (sem->AccessControl() != ast::AccessControl::kInvalid) { @@ -806,8 +807,10 @@ auto* type = TypeOf(expr->idx_expr()); idx_id = GenerateLoadIfNeeded(type, idx_id); - // If the source is a pointer, we access chain into it. - if (info->source_type->Is<sem::Pointer>()) { + // If the source is a reference, we access chain into it. + // In the future, pointers may support access-chaining. + // See https://github.com/gpuweb/gpuweb/pull/1580 + if (info->source_type->Is<sem::Reference>()) { info->access_chain_indices.push_back(idx_id); info->source_type = TypeOf(expr); return true; @@ -870,7 +873,7 @@ if (auto* access = expr_sem->As<sem::StructMemberAccess>()) { uint32_t idx = access->Member()->Index(); - if (info->source_type->Is<sem::Pointer>()) { + if (info->source_type->Is<sem::Reference>()) { auto idx_id = GenerateConstantIfNeeded(ScalarConstant::U32(idx)); if (idx_id == 0) { return 0; @@ -903,7 +906,7 @@ // Single element swizzle is either an access chain or a composite extract auto& indices = swizzle->Indices(); if (indices.size() == 1) { - if (info->source_type->Is<sem::Pointer>()) { + if (info->source_type->Is<sem::Reference>()) { auto idx_id = GenerateConstantIfNeeded(ScalarConstant::U32(indices[0])); if (idx_id == 0) { return 0; @@ -954,7 +957,7 @@ } info->source_id = GenerateLoadIfNeeded(expr_type, extract_id); - info->source_type = expr_type->UnwrapPtr(); + info->source_type = expr_type->UnwrapRef(); info->access_chain_indices.clear(); } @@ -1022,25 +1025,31 @@ // If our initial access is into a non-pointer array, and either has a // non-scalar element type or the accessor uses a non-literal index, then we // need to load that array into a variable in order to access chain into it. - // TODO(jrprice): The non-scalar part shouldn't be necessary, but is tied to - // how the Resolver currently determines the type of these expression. This - // should be fixed when proper support for ptr/ref types is implemented. - if (auto* array = accessors[0]->As<ast::ArrayAccessorExpression>()) { - auto* ary_res_type = TypeOf(array->array())->As<sem::Array>(); - if (ary_res_type && - (!ary_res_type->ElemType()->is_scalar() || - !array->idx_expr()->Is<ast::ScalarConstructorExpression>())) { - // Wrap the source type in a pointer to function storage. - auto ptr = - builder_.ty.pointer(ary_res_type, ast::StorageClass::kFunction); - auto result_type_id = GenerateTypeIfNeeded(ptr); + + // TODO(bclayton): The requirement for scalar element types is because of + // arrays-of-arrays - this logic only considers whether the root index is + // compile-time-constant, and not whether there are any dynamic, inner-array + // indexing being performed. Instead of trying to do complex hoisting in this + // writer, move this hoisting into the transform::Spirv sanitizer. + + bool needs_load = false; // Was the expression hoist to a temporary variable? + if (auto* access = accessors[0]->As<ast::ArrayAccessorExpression>()) { + auto* array = TypeOf(access->array())->As<sem::Array>(); + bool trivial_indexing = + array && array->ElemType()->is_scalar() && + access->idx_expr()->Is<ast::ScalarConstructorExpression>(); + if (array && !trivial_indexing) { + // Wrap the source type in a reference to function storage. + auto* ref = + builder_.create<sem::Reference>(array, ast::StorageClass::kFunction); + auto result_type_id = GenerateTypeIfNeeded(ref); if (result_type_id == 0) { return 0; } auto ary_result = result_op(); - auto init = GenerateConstantNullIfNeeded(ary_res_type); + auto init = GenerateConstantNullIfNeeded(array); // If we're access chaining into an array then we must be in a function push_function_var( @@ -1054,7 +1063,8 @@ } info.source_id = ary_result.to_i(); - info.source_type = ptr; + info.source_type = ref; + needs_load = true; } } @@ -1076,17 +1086,12 @@ } if (!info.access_chain_indices.empty()) { - bool needs_load = false; - auto* ptr = TypeOf(expr); - if (!ptr->Is<sem::Pointer>()) { - // We are performing an access chain but the final result is not a - // pointer, so we need to perform a load to get it. This happens when we - // have to copy the source expression into a function variable. - ptr = builder_.ty.pointer(ptr, ast::StorageClass::kFunction); - needs_load = true; + auto* type = TypeOf(expr); + if (needs_load) { + type = + builder_.create<sem::Reference>(type, ast::StorageClass::kFunction); } - - auto result_type_id = GenerateTypeIfNeeded(ptr); + auto result_type_id = GenerateTypeIfNeeded(type); if (result_type_id == 0) { return 0; } @@ -1107,7 +1112,7 @@ // Load from the access chain result if required. if (needs_load) { - info.source_id = GenerateLoadIfNeeded(ptr, result_id); + info.source_id = GenerateLoadIfNeeded(type, result_id); } } @@ -1127,16 +1132,18 @@ } uint32_t Builder::GenerateLoadIfNeeded(const sem::Type* type, uint32_t id) { - if (!type->Is<sem::Pointer>()) { + if (auto* ref = type->As<sem::Reference>()) { + type = ref->StoreType(); + } else { return id; } - auto type_id = GenerateTypeIfNeeded(type->UnwrapPtr()); + auto type_id = GenerateTypeIfNeeded(type); auto result = result_op(); auto result_id = result.to_i(); if (!push_function_inst(spv::Op::OpLoad, {Operand::Int(type_id), result, Operand::Int(id)})) { - return false; + return 0; } return result_id; } @@ -1149,6 +1156,27 @@ if (val_id == 0) { return 0; } + + spv::Op op = spv::Op::OpNop; + switch (expr->op()) { + case ast::UnaryOp::kNegation: + if (TypeOf(expr)->is_float_scalar_or_vector()) { + op = spv::Op::OpFNegate; + } else { + op = spv::Op::OpSNegate; + } + break; + case ast::UnaryOp::kNot: + op = spv::Op::OpLogicalNot; + break; + case ast::UnaryOp::kAddressOf: + case ast::UnaryOp::kIndirection: + // Address-of converts a reference to a pointer, and dereference converts + // a pointer to a reference. These are the same thing in SPIR-V, so this + // is a no-op. + return val_id; + } + val_id = GenerateLoadIfNeeded(TypeOf(expr->expr()), val_id); auto type_id = GenerateTypeIfNeeded(TypeOf(expr)); @@ -1156,21 +1184,6 @@ return 0; } - spv::Op op = spv::Op::OpNop; - if (expr->op() == ast::UnaryOp::kNegation) { - if (TypeOf(expr)->is_float_scalar_or_vector()) { - op = spv::Op::OpFNegate; - } else { - op = spv::Op::OpSNegate; - } - } else if (expr->op() == ast::UnaryOp::kNot) { - op = spv::Op::OpLogicalNot; - } - if (op == spv::Op::OpNop) { - error_ = "invalid unary op type"; - return 0; - } - if (!push_function_inst( op, {Operand::Int(type_id), result, Operand::Int(val_id)})) { return false; @@ -1218,7 +1231,7 @@ } auto* tc = constructor->As<ast::TypeConstructorExpression>(); - auto* result_type = TypeOf(tc)->UnwrapAll(); + auto* result_type = TypeOf(tc)->UnwrapRef(); for (size_t i = 0; i < tc->values().size(); ++i) { auto* e = tc->values()[i]; @@ -1246,17 +1259,17 @@ continue; } - const sem::Type* subtype = result_type->UnwrapAll(); + const sem::Type* subtype = result_type->UnwrapRef(); if (auto* vec = subtype->As<sem::Vector>()) { - subtype = vec->type()->UnwrapAll(); + subtype = vec->type(); } else if (auto* mat = subtype->As<sem::Matrix>()) { - subtype = mat->type()->UnwrapAll(); + subtype = mat->type(); } else if (auto* arr = subtype->As<sem::Array>()) { - subtype = arr->ElemType()->UnwrapAll(); + subtype = arr->ElemType(); } else if (auto* str = subtype->As<sem::Struct>()) { - subtype = str->Members()[i]->Type()->UnwrapAll(); + subtype = str->Members()[i]->Type(); } - if (subtype != TypeOf(sc)->UnwrapAll()) { + if (subtype != TypeOf(sc)->UnwrapRef()) { return false; } } @@ -1272,13 +1285,13 @@ // Generate the zero initializer if there are no values provided. if (values.empty()) { - return GenerateConstantNullIfNeeded(result_type->UnwrapPtr()); + return GenerateConstantNullIfNeeded(result_type->UnwrapRef()); } std::ostringstream out; out << "__const_" << init->type()->FriendlyName(builder_.Symbols()) << "_"; - result_type = result_type->UnwrapAll(); + result_type = result_type->UnwrapRef(); bool constructor_is_const = is_constructor_const(init, is_global_init); if (has_error()) { return 0; @@ -1288,7 +1301,7 @@ if (auto* res_vec = result_type->As<sem::Vector>()) { if (res_vec->type()->is_scalar()) { - auto* value_type = TypeOf(values[0])->UnwrapAll(); + auto* value_type = TypeOf(values[0])->UnwrapRef(); if (auto* val_vec = value_type->As<sem::Vector>()) { if (val_vec->type()->is_scalar()) { can_cast_or_copy = res_vec->size() == val_vec->size(); @@ -1327,7 +1340,7 @@ return 0; } - auto* value_type = TypeOf(e)->UnwrapPtr(); + auto* value_type = TypeOf(e)->UnwrapRef(); // If the result and value types are the same we can just use the object. // If the result is not a vector then we should have validated that the // value type is a correctly sized vector so we can just use it directly. @@ -1444,7 +1457,7 @@ } val_id = GenerateLoadIfNeeded(TypeOf(from_expr), val_id); - auto* from_type = TypeOf(from_expr)->UnwrapPtr(); + auto* from_type = TypeOf(from_expr)->UnwrapRef(); spv::Op op = spv::Op::OpNop; if ((from_type->Is<sem::I32>() && to_type->Is<sem::F32>()) || @@ -1728,8 +1741,8 @@ // Handle int and float and the vectors of those types. Other types // should have been rejected by validation. - auto* lhs_type = TypeOf(expr->lhs())->UnwrapAll(); - auto* rhs_type = TypeOf(expr->rhs())->UnwrapAll(); + auto* lhs_type = TypeOf(expr->lhs())->UnwrapRef(); + auto* rhs_type = TypeOf(expr->rhs())->UnwrapRef(); bool lhs_is_float_or_vec = lhs_type->is_float_scalar_or_vector(); bool lhs_is_unsigned = lhs_type->is_unsigned_scalar_or_vector(); @@ -1904,13 +1917,18 @@ } ops.push_back(Operand::Int(func_id)); - for (auto* param : expr->params()) { - auto id = GenerateExpression(param); + size_t arg_idx = 0; + for (auto* arg : expr->params()) { + auto id = GenerateExpression(arg); if (id == 0) { return 0; } - id = GenerateLoadIfNeeded(TypeOf(param), id); + id = GenerateLoadIfNeeded(TypeOf(arg), id); + if (id == 0) { + return 0; + } ops.push_back(Operand::Int(id)); + arg_idx++; } if (!push_function_inst(spv::Op::OpFunctionCall, std::move(ops))) { @@ -1982,7 +2000,7 @@ } params.push_back(Operand::Int(struct_id)); - auto* type = TypeOf(accessor->structure())->UnwrapAll(); + auto* type = TypeOf(accessor->structure())->UnwrapRef(); if (!type->Is<sem::Struct>()) { error_ = "invalid type (" + type->type_name() + ") for runtime array length"; @@ -2134,7 +2152,7 @@ TINT_ICE(builder_.Diagnostics()) << "missing texture argument"; } - auto* texture_type = TypeOf(texture)->UnwrapAll()->As<sem::Texture>(); + auto* texture_type = TypeOf(texture)->UnwrapRef()->As<sem::Texture>(); auto op = spv::Op::OpNop; @@ -2580,8 +2598,8 @@ val_id = GenerateLoadIfNeeded(TypeOf(expr->expr()), val_id); // Bitcast does not allow same types, just emit a CopyObject - auto* to_type = TypeOf(expr)->UnwrapPtr(); - auto* from_type = TypeOf(expr->expr())->UnwrapPtr(); + auto* to_type = TypeOf(expr)->UnwrapRef(); + auto* from_type = TypeOf(expr->expr())->UnwrapRef(); if (to_type->type_name() == from_type->type_name()) { if (!push_function_inst( spv::Op::OpCopyObject, @@ -2932,84 +2950,97 @@ return 0; } - auto val = type_name_to_id_.find(type->type_name()); - if (val != type_name_to_id_.end()) { - return val->second; - } - - auto result = result_op(); - auto id = result.to_i(); - if (auto* arr = type->As<sem::Array>()) { - if (!GenerateArrayType(arr, result)) { - return 0; - } - } else if (type->Is<sem::Bool>()) { - push_type(spv::Op::OpTypeBool, {result}); - } else if (type->Is<sem::F32>()) { - push_type(spv::Op::OpTypeFloat, {result, Operand::Int(32)}); - } else if (type->Is<sem::I32>()) { - push_type(spv::Op::OpTypeInt, {result, Operand::Int(32), Operand::Int(1)}); - } else if (auto* mat = type->As<sem::Matrix>()) { - if (!GenerateMatrixType(mat, result)) { - return 0; - } - } else if (auto* ptr = type->As<sem::Pointer>()) { - if (!GeneratePointerType(ptr, result)) { - return 0; - } - } else if (auto* str = type->As<sem::Struct>()) { - if (!GenerateStructType(str, result)) { - return 0; - } - } else if (type->Is<sem::U32>()) { - push_type(spv::Op::OpTypeInt, {result, Operand::Int(32), Operand::Int(0)}); - } else if (auto* vec = type->As<sem::Vector>()) { - if (!GenerateVectorType(vec, result)) { - return 0; - } - } else if (type->Is<sem::Void>()) { - push_type(spv::Op::OpTypeVoid, {result}); - } else if (auto* tex = type->As<sem::Texture>()) { - if (!GenerateTextureType(tex, result)) { - return 0; - } - - if (auto* st = tex->As<sem::StorageTexture>()) { - // Register all three access types of StorageTexture names. In SPIR-V, we - // must output a single type, while the variable is annotated with the - // access type. Doing this ensures we de-dupe. - type_name_to_id_[builder_ - .create<sem::StorageTexture>( - st->dim(), st->image_format(), - ast::AccessControl::kReadOnly, st->type()) - ->type_name()] = id; - type_name_to_id_[builder_ - .create<sem::StorageTexture>( - st->dim(), st->image_format(), - ast::AccessControl::kWriteOnly, st->type()) - ->type_name()] = id; - type_name_to_id_[builder_ - .create<sem::StorageTexture>( - st->dim(), st->image_format(), - ast::AccessControl::kReadWrite, st->type()) - ->type_name()] = id; - } - - } else if (type->Is<sem::Sampler>()) { - push_type(spv::Op::OpTypeSampler, {result}); - - // Register both of the sampler type names. In SPIR-V they're the same - // sampler type, so we need to match that when we do the dedup check. - type_name_to_id_["__sampler_sampler"] = id; - type_name_to_id_["__sampler_comparison"] = id; - + // Pointers and References both map to a SPIR-V pointer type. + // Transform a Reference to a Pointer to prevent these having duplicated + // definitions in the generated SPIR-V. Note that nested references are not + // legal, so only considering the top-level type is fine. + std::string type_name; + if (auto* ref = type->As<sem::Reference>()) { + type_name = sem::Pointer(ref->StoreType(), ref->StorageClass()).type_name(); } else { - error_ = "unable to convert type: " + type->type_name(); - return 0; + type_name = type->type_name(); } - type_name_to_id_[type->type_name()] = id; - return id; + return utils::GetOrCreate(type_name_to_id_, type_name, [&]() -> uint32_t { + auto result = result_op(); + auto id = result.to_i(); + if (auto* arr = type->As<sem::Array>()) { + if (!GenerateArrayType(arr, result)) { + return 0; + } + } else if (type->Is<sem::Bool>()) { + push_type(spv::Op::OpTypeBool, {result}); + } else if (type->Is<sem::F32>()) { + push_type(spv::Op::OpTypeFloat, {result, Operand::Int(32)}); + } else if (type->Is<sem::I32>()) { + push_type(spv::Op::OpTypeInt, + {result, Operand::Int(32), Operand::Int(1)}); + } else if (auto* mat = type->As<sem::Matrix>()) { + if (!GenerateMatrixType(mat, result)) { + return 0; + } + } else if (auto* ptr = type->As<sem::Pointer>()) { + if (!GeneratePointerType(ptr, result)) { + return 0; + } + } else if (auto* ref = type->As<sem::Reference>()) { + if (!GenerateReferenceType(ref, result)) { + return 0; + } + } else if (auto* str = type->As<sem::Struct>()) { + if (!GenerateStructType(str, result)) { + return 0; + } + } else if (type->Is<sem::U32>()) { + push_type(spv::Op::OpTypeInt, + {result, Operand::Int(32), Operand::Int(0)}); + } else if (auto* vec = type->As<sem::Vector>()) { + if (!GenerateVectorType(vec, result)) { + return 0; + } + } else if (type->Is<sem::Void>()) { + push_type(spv::Op::OpTypeVoid, {result}); + } else if (auto* tex = type->As<sem::Texture>()) { + if (!GenerateTextureType(tex, result)) { + return 0; + } + + if (auto* st = tex->As<sem::StorageTexture>()) { + // Register all three access types of StorageTexture names. In SPIR-V, + // we must output a single type, while the variable is annotated with + // the access type. Doing this ensures we de-dupe. + type_name_to_id_[builder_ + .create<sem::StorageTexture>( + st->dim(), st->image_format(), + ast::AccessControl::kReadOnly, st->type()) + ->type_name()] = id; + type_name_to_id_[builder_ + .create<sem::StorageTexture>( + st->dim(), st->image_format(), + ast::AccessControl::kWriteOnly, st->type()) + ->type_name()] = id; + type_name_to_id_[builder_ + .create<sem::StorageTexture>( + st->dim(), st->image_format(), + ast::AccessControl::kReadWrite, st->type()) + ->type_name()] = id; + } + + } else if (type->Is<sem::Sampler>()) { + push_type(spv::Op::OpTypeSampler, {result}); + + // Register both of the sampler type names. In SPIR-V they're the same + // sampler type, so we need to match that when we do the dedup check. + type_name_to_id_["__sampler_sampler"] = id; + type_name_to_id_["__sampler_comparison"] = id; + + } else { + error_ = "unable to convert type: " + type->type_name(); + return 0; + } + + return id; + }); } // TODO(tommek): Cover multisampled textures here when they're included in AST @@ -3131,8 +3162,8 @@ bool Builder::GeneratePointerType(const sem::Pointer* ptr, const Operand& result) { - auto pointee_id = GenerateTypeIfNeeded(ptr->StoreType()); - if (pointee_id == 0) { + auto subtype_id = GenerateTypeIfNeeded(ptr->StoreType()); + if (subtype_id == 0) { return false; } @@ -3143,7 +3174,26 @@ } push_type(spv::Op::OpTypePointer, - {result, Operand::Int(stg_class), Operand::Int(pointee_id)}); + {result, Operand::Int(stg_class), Operand::Int(subtype_id)}); + + return true; +} + +bool Builder::GenerateReferenceType(const sem::Reference* ref, + const Operand& result) { + auto subtype_id = GenerateTypeIfNeeded(ref->StoreType()); + if (subtype_id == 0) { + return false; + } + + auto stg_class = ConvertStorageClass(ref->StorageClass()); + if (stg_class == SpvStorageClassMax) { + error_ = "invalid storage class for reference"; + return false; + } + + push_type(spv::Op::OpTypePointer, + {result, Operand::Int(stg_class), Operand::Int(subtype_id)}); return true; }
diff --git a/src/writer/spirv/builder.h b/src/writer/spirv/builder.h index 53c917b..98675ff 100644 --- a/src/writer/spirv/builder.h +++ b/src/writer/spirv/builder.h
@@ -43,6 +43,7 @@ // Forward declarations namespace sem { class Call; +class Reference; } // namespace sem namespace writer { @@ -66,7 +67,7 @@ /// result_type of the current source defined above. const sem::Type* source_type; /// A list of access chain indices to emit. Note, we _only_ have access - /// chain indices if the source is pointer. + /// chain indices if the source is reference. std::vector<uint32_t> access_chain_indices; }; @@ -411,7 +412,7 @@ /// Geneates an OpLoad /// @param type the type to load /// @param id the variable id to load - /// @returns the ID of the loaded value or `id` if type is not a pointer + /// @returns the ID of the loaded value or `id` if type is not a reference uint32_t GenerateLoadIfNeeded(const sem::Type* type, uint32_t id); /// Generates an OpStore. Emits an error and returns false if we're /// currently outside a function. @@ -443,6 +444,11 @@ /// @param result the result operand /// @returns true if the pointer was successfully generated bool GeneratePointerType(const sem::Pointer* ptr, const Operand& result); + /// Generates a reference type declaration + /// @param ref the reference type to generate + /// @param result the result operand + /// @returns true if the reference was successfully generated + bool GenerateReferenceType(const sem::Reference* ref, const Operand& result); /// Generates a vector type declaration /// @param struct_type the vector to generate /// @param result the result operand
diff --git a/src/writer/spirv/builder_accessor_expression_test.cc b/src/writer/spirv/builder_accessor_expression_test.cc index 6962231..6e36a06 100644 --- a/src/writer/spirv/builder_accessor_expression_test.cc +++ b/src/writer/spirv/builder_accessor_expression_test.cc
@@ -766,7 +766,7 @@ b.push_function(Function{}); ASSERT_TRUE(b.GenerateFunctionVariable(var)) << b.error(); - EXPECT_EQ(b.GenerateAccessorExpression(expr), 18u) << b.error(); + EXPECT_EQ(b.GenerateAccessorExpression(expr), 19u) << b.error(); EXPECT_EQ(DumpInstructions(b.types()), R"(%3 = OpTypeFloat 32 %2 = OpTypeVector %3 2 @@ -791,6 +791,57 @@ EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()), R"(OpStore %14 %12 %18 = OpAccessChain %17 %14 %16 +%19 = OpLoad %2 %18 +)"); +} + +TEST_F(BuilderTest, Accessor_Array_Of_Array_Of_f32) { + // let pos : array<array<f32, 2>, 3> = array<vec2<f32, 2>, 3>( + // array<f32, 2>(0.0, 0.5), + // array<f32, 2>(-0.5, -0.5), + // array<f32, 2>(0.5, -0.5)); + // pos[2][1] + + auto* var = + Const("pos", ty.array(ty.vec2<f32>(), 3), + Construct(ty.array(ty.vec2<f32>(), 3), vec2<f32>(0.0f, 0.5f), + vec2<f32>(-0.5f, -0.5f), vec2<f32>(0.5f, -0.5f))); + + auto* expr = IndexAccessor(IndexAccessor("pos", 2u), 1u); + WrapInFunction(var, expr); + + spirv::Builder& b = Build(); + + b.push_function(Function{}); + ASSERT_TRUE(b.GenerateFunctionVariable(var)) << b.error(); + EXPECT_EQ(b.GenerateAccessorExpression(expr), 21u) << b.error(); + + EXPECT_EQ(DumpInstructions(b.types()), R"(%3 = OpTypeFloat 32 +%2 = OpTypeVector %3 2 +%4 = OpTypeInt 32 0 +%5 = OpConstant %4 3 +%1 = OpTypeArray %2 %5 +%6 = OpConstant %3 0 +%7 = OpConstant %3 0.5 +%8 = OpConstantComposite %2 %6 %7 +%9 = OpConstant %3 -0.5 +%10 = OpConstantComposite %2 %9 %9 +%11 = OpConstantComposite %2 %7 %9 +%12 = OpConstantComposite %1 %8 %10 %11 +%13 = OpTypePointer Function %1 +%15 = OpConstantNull %1 +%16 = OpConstant %4 2 +%17 = OpConstant %4 1 +%19 = OpTypePointer Function %3 +)"); + EXPECT_EQ(DumpInstructions(b.functions()[0].variables()), + R"(%14 = OpVariable %13 Function %15 +)"); + EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()), + R"(OpStore %14 %12 +%18 = OpCompositeExtract %3 %14 1 +%20 = OpAccessChain %19 %18 %16 +%21 = OpLoad %3 %20 )"); }
diff --git a/src/writer/spirv/builder_ident_expression_test.cc b/src/writer/spirv/builder_ident_expression_test.cc index 821f9e5..c57b44a 100644 --- a/src/writer/spirv/builder_ident_expression_test.cc +++ b/src/writer/spirv/builder_ident_expression_test.cc
@@ -90,7 +90,7 @@ } TEST_F(BuilderTest, IdentifierExpression_FunctionVar) { - auto* v = Var("var", ty.f32(), ast::StorageClass::kNone); + auto* v = Var("var", ty.f32(), ast::StorageClass::kFunction); auto* expr = Expr("var"); WrapInFunction(v, expr);
diff --git a/src/writer/spirv/builder_intrinsic_test.cc b/src/writer/spirv/builder_intrinsic_test.cc index bca5f4a..d6d8156 100644 --- a/src/writer/spirv/builder_intrinsic_test.cc +++ b/src/writer/spirv/builder_intrinsic_test.cc
@@ -1262,7 +1262,7 @@ TEST_F(IntrinsicBuilderTest, Call_Modf) { auto* out = Var("out", ty.vec2<f32>()); - auto* expr = Call("modf", vec2<f32>(1.0f, 2.0f), "out"); + auto* expr = Call("modf", vec2<f32>(1.0f, 2.0f), AddressOf("out")); Func("a_func", ast::VariableList{}, ty.void_(), ast::StatementList{ Decl(out), @@ -1306,7 +1306,7 @@ TEST_F(IntrinsicBuilderTest, Call_Frexp) { auto* out = Var("out", ty.vec2<i32>()); - auto* expr = Call("frexp", vec2<f32>(1.0f, 2.0f), "out"); + auto* expr = Call("frexp", vec2<f32>(1.0f, 2.0f), AddressOf("out")); Func("a_func", ast::VariableList{}, ty.void_(), ast::StatementList{ Decl(out),