spirv: Add calls to MaybeCreateTypename() typ::Types originating from ConvertType() will hold direct pointers to ast::Struct and ast::Aliase. These must not be used directly. Instead TypeNames should be created to refer to these. Bug: tint:724 Change-Id: I7f511e0da56e5d3ac587cdb22b0baf38474bed1e Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49746 Commit-Queue: Ben Clayton <bclayton@google.com> Reviewed-by: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index 5b17ca5..0027042 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc
@@ -3358,7 +3358,8 @@ operands.emplace_back(MakeOperand(inst, iarg).expr); } return {ast_type, create<ast::TypeConstructorExpression>( - Source{}, ast_type, std::move(operands))}; + Source{}, builder_.ty.MaybeCreateTypename(ast_type), + std::move(operands))}; } if (opcode == SpvOpCompositeExtract) { @@ -3885,7 +3886,8 @@ } } return {result_type, - create<ast::TypeConstructorExpression>(source, result_type, values)}; + create<ast::TypeConstructorExpression>( + source, builder_.ty.MaybeCreateTypename(result_type), values)}; } bool FunctionEmitter::RegisterSpecialBuiltInVariables() { @@ -4230,8 +4232,9 @@ ast::ExpressionList params; params.push_back(arg_expr.expr); TypedExpression result{ - expr_type, create<ast::TypeConstructorExpression>(Source{}, expr_type, - std::move(params))}; + expr_type, create<ast::TypeConstructorExpression>( + Source{}, builder_.ty.MaybeCreateTypename(expr_type), + std::move(params))}; if (requested_type == expr_type) { return result; @@ -4660,7 +4663,7 @@ if (is_non_dref_sample || (opcode == SpvOpImageFetch)) { value = create<ast::TypeConstructorExpression>( Source{}, - result_type, // a vec4 + builder_.ty.MaybeCreateTypename(result_type), // a vec4 ast::ExpressionList{ value, parser_impl_.MakeNullValue(result_component_type), parser_impl_.MakeNullValue(result_component_type), @@ -4738,7 +4741,8 @@ auto result_type = parser_impl_.ConvertType(inst.type_id()); TypedExpression expr = { result_type, - create<ast::TypeConstructorExpression>(Source{}, result_type, exprs)}; + create<ast::TypeConstructorExpression>( + Source{}, builder_.ty.MaybeCreateTypename(result_type), exprs)}; return EmitConstDefOrWriteToHoistedVar(inst, expr); } case SpvOpImageQueryLod: @@ -4760,7 +4764,8 @@ // returns i32. If they aren't the same then convert the result. if (result_type != builder_.ty.i32()) { ast_expr = create<ast::TypeConstructorExpression>( - Source{}, result_type, ast::ExpressionList{ast_expr}); + Source{}, builder_.ty.MaybeCreateTypename(result_type), + ast::ExpressionList{ast_expr}); } TypedExpression expr{result_type, ast_expr}; return EmitConstDefOrWriteToHoistedVar(inst, expr); @@ -5069,11 +5074,12 @@ Source{}, ast::BinaryOp::kMultiply, row_factor, column_factor); result_row.push_back(elem); } - result_columns.push_back( - create<ast::TypeConstructorExpression>(Source{}, col_ty, result_row)); + result_columns.push_back(create<ast::TypeConstructorExpression>( + Source{}, builder_.ty.MaybeCreateTypename(col_ty), result_row)); } - return {result_ty, create<ast::TypeConstructorExpression>(Source{}, result_ty, - result_columns)}; + return {result_ty, create<ast::TypeConstructorExpression>( + Source{}, builder_.ty.MaybeCreateTypename(result_ty), + result_columns)}; } bool FunctionEmitter::MakeVectorInsertDynamic( @@ -5102,8 +5108,9 @@ auto registered_temp_name = builder_.Symbols().Register(temp_name); auto* temp_var = create<ast::Variable>( - Source{}, registered_temp_name, ast::StorageClass::kFunction, ast_type, - false, src_vector.expr, ast::DecorationList{}); + Source{}, registered_temp_name, ast::StorageClass::kFunction, + builder_.ty.MaybeCreateTypename(ast_type), false, src_vector.expr, + ast::DecorationList{}); AddStatement(create<ast::VariableDeclStatement>(Source{}, temp_var)); auto* lhs = create<ast::ArrayAccessorExpression>( @@ -5148,8 +5155,9 @@ auto registered_temp_name = builder_.Symbols().Register(temp_name); auto* temp_var = create<ast::Variable>( - Source{}, registered_temp_name, ast::StorageClass::kFunction, ast_type, - false, src_composite.expr, ast::DecorationList{}); + Source{}, registered_temp_name, ast::StorageClass::kFunction, + builder_.ty.MaybeCreateTypename(ast_type), false, src_composite.expr, + ast::DecorationList{}); AddStatement(create<ast::VariableDeclStatement>(Source{}, temp_var)); TypedExpression seed_expr{ast_type, create<ast::IdentifierExpression>(
diff --git a/src/reader/spirv/function_composite_test.cc b/src/reader/spirv/function_composite_test.cc index 022abfa..89fa243 100644 --- a/src/reader/spirv/function_composite_test.cc +++ b/src/reader/spirv/function_composite_test.cc
@@ -221,7 +221,7 @@ __struct_S { TypeConstructor[not set]{ - __struct_S + __type_name_S TypeConstructor[not set]{ __vec_2__f32 ScalarConstructor[not set]{50.000000} @@ -833,6 +833,23 @@ auto body_str = ToString(p->builder(), fe.ast_body()); EXPECT_THAT(body_str, HasSubstr(R"(VariableDeclStatement{ Variable{ + x_35 + function + __struct_S + } +} +VariableDeclStatement{ + VariableConst{ + x_1 + none + __struct_S + { + Identifier[not set]{x_35} + } + } +} +VariableDeclStatement{ + Variable{ x_2_1 function __struct_S @@ -890,6 +907,30 @@ auto body_str = ToString(p->builder(), fe.ast_body()); EXPECT_THAT(body_str, HasSubstr(R"(VariableDeclStatement{ Variable{ + x_40 + function + __struct_S_2 + } +} +VariableDeclStatement{ + Variable{ + x_41 + function + __struct_S_2 + } +} +VariableDeclStatement{ + VariableConst{ + x_1 + none + __struct_S_1 + { + Identifier[not set]{x_40} + } + } +} +VariableDeclStatement{ + Variable{ x_2_1 function __struct_S_1 @@ -915,6 +956,43 @@ } } } +VariableDeclStatement{ + VariableConst{ + x_3 + none + __struct_S_2 + { + Identifier[not set]{x_41} + } + } +} +VariableDeclStatement{ + Variable{ + x_4_1 + function + __struct_S_2 + { + Identifier[not set]{x_3} + } + } +} +Assignment{ + MemberAccessor[not set]{ + Identifier[not set]{x_4_1} + Identifier[not set]{rithm} + } + ScalarConstructor[not set]{10u} +} +VariableDeclStatement{ + VariableConst{ + x_4 + none + __struct_S_2 + { + Identifier[not set]{x_4_1} + } + } +} )")) << body_str; EXPECT_THAT(body_str, HasSubstr(R"(VariableDeclStatement{ Variable{ @@ -986,6 +1064,23 @@ auto body_str = ToString(p->builder(), fe.ast_body()); EXPECT_THAT(body_str, HasSubstr(R"(VariableDeclStatement{ Variable{ + x_37 + function + __struct_S_1 + } +} +VariableDeclStatement{ + VariableConst{ + x_1 + none + __struct_S_1 + { + Identifier[not set]{x_37} + } + } +} +VariableDeclStatement{ + Variable{ x_2_1 function __struct_S_1
diff --git a/src/reader/spirv/function_misc_test.cc b/src/reader/spirv/function_misc_test.cc index 5f747b5..b523d8b 100644 --- a/src/reader/spirv/function_misc_test.cc +++ b/src/reader/spirv/function_misc_test.cc
@@ -412,7 +412,7 @@ __struct_S { TypeConstructor[not set]{ - __struct_S + __type_name_S ScalarConstructor[not set]{false} ScalarConstructor[not set]{0u} ScalarConstructor[not set]{0}
diff --git a/src/reader/spirv/function_var_test.cc b/src/reader/spirv/function_var_test.cc index 6762822..7bf5b0e 100644 --- a/src/reader/spirv/function_var_test.cc +++ b/src/reader/spirv/function_var_test.cc
@@ -470,7 +470,7 @@ __alias_Arr__array__u32_2_stride_16 { TypeConstructor[not set]{ - __alias_Arr__array__u32_2_stride_16 + __type_name_Arr ScalarConstructor[not set]{1u} ScalarConstructor[not set]{2u} } @@ -540,7 +540,7 @@ __alias_Arr__array__u32_2_stride_16 { TypeConstructor[not set]{ - __alias_Arr__array__u32_2_stride_16 + __type_name_Arr ScalarConstructor[not set]{0u} ScalarConstructor[not set]{0u} } @@ -575,7 +575,7 @@ __struct_S { TypeConstructor[not set]{ - __struct_S + __type_name_S ScalarConstructor[not set]{1u} ScalarConstructor[not set]{1.500000} TypeConstructor[not set]{ @@ -615,7 +615,7 @@ __struct_S { TypeConstructor[not set]{ - __struct_S + __type_name_S ScalarConstructor[not set]{0u} ScalarConstructor[not set]{0.000000} TypeConstructor[not set]{
diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index c440dc3..6820ba0 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc
@@ -1442,13 +1442,9 @@ } std::string name = namer_.Name(id); - return create<ast::Variable>(Source{}, // source - builder_.Symbols().Register(name), // symbol - sc, // storage_class - type, // type - is_const, // is_const - constructor, // constructor - decorations); // decorations + return create<ast::Variable>(Source{}, builder_.Symbols().Register(name), sc, + builder_.ty.MaybeCreateTypename(type), is_const, + constructor, decorations); } TypedExpression ParserImpl::MakeConstantExpression(uint32_t id) { @@ -1530,8 +1526,9 @@ ast_components.emplace_back(ast_component.expr); } return {original_ast_type, - create<ast::TypeConstructorExpression>(Source{}, original_ast_type, - std::move(ast_components))}; + create<ast::TypeConstructorExpression>( + Source{}, builder_.ty.MaybeCreateTypename(original_ast_type), + std::move(ast_components))}; } auto* spirv_null_const = spirv_const->AsNullConstant(); if (spirv_null_const != nullptr) { @@ -1583,8 +1580,9 @@ for (size_t i = 0; i < vec_ty->size(); ++i) { ast_components.emplace_back(MakeNullValue(typ::Call_type(vec_ty))); } - return create<ast::TypeConstructorExpression>(Source{}, type, - std::move(ast_components)); + return create<ast::TypeConstructorExpression>( + Source{}, builder_.ty.MaybeCreateTypename(type), + std::move(ast_components)); } if (auto mat_ty = typ::As<typ::Matrix>(type)) { // Matrix components are columns @@ -1593,24 +1591,27 @@ for (size_t i = 0; i < mat_ty->columns(); ++i) { ast_components.emplace_back(MakeNullValue(column_ty)); } - return create<ast::TypeConstructorExpression>(Source{}, type, - std::move(ast_components)); + return create<ast::TypeConstructorExpression>( + Source{}, builder_.ty.MaybeCreateTypename(type), + std::move(ast_components)); } if (auto arr_ty = typ::As<typ::Array>(type)) { ast::ExpressionList ast_components; for (size_t i = 0; i < arr_ty->size(); ++i) { ast_components.emplace_back(MakeNullValue(typ::Call_type(arr_ty))); } - return create<ast::TypeConstructorExpression>(Source{}, original_type, - std::move(ast_components)); + return create<ast::TypeConstructorExpression>( + Source{}, builder_.ty.MaybeCreateTypename(original_type), + std::move(ast_components)); } if (auto struct_ty = typ::As<typ::Struct>(type)) { ast::ExpressionList ast_components; for (auto* member : struct_ty.ast->members()) { ast_components.emplace_back(MakeNullValue(member->type())); } - return create<ast::TypeConstructorExpression>(Source{}, original_type, - std::move(ast_components)); + return create<ast::TypeConstructorExpression>( + Source{}, builder_.ty.MaybeCreateTypename(original_type), + std::move(ast_components)); } Fail() << "can't make null value for type: " << type->type_name(); return nullptr;
diff --git a/src/reader/spirv/parser_impl_module_var_test.cc b/src/reader/spirv/parser_impl_module_var_test.cc index 67ca1f6..bb5d9f4 100644 --- a/src/reader/spirv/parser_impl_module_var_test.cc +++ b/src/reader/spirv/parser_impl_module_var_test.cc
@@ -1411,7 +1411,7 @@ __struct_S { TypeConstructor[not set]{ - __struct_S + __type_name_S ScalarConstructor[not set]{1u} ScalarConstructor[not set]{1.500000} TypeConstructor[not set]{ @@ -1440,7 +1440,7 @@ __struct_S { TypeConstructor[not set]{ - __struct_S + __type_name_S ScalarConstructor[not set]{0u} ScalarConstructor[not set]{0.000000} TypeConstructor[not set]{ @@ -1469,7 +1469,7 @@ __struct_S { TypeConstructor[not set]{ - __struct_S + __type_name_S ScalarConstructor[not set]{0u} ScalarConstructor[not set]{0.000000} TypeConstructor[not set]{