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>
This commit is contained in:
Ben Clayton 2021-05-04 18:13:11 +00:00 committed by Commit Bot service account
parent e5b315084d
commit 3e5e4fd8e5
6 changed files with 145 additions and 41 deletions

View File

@ -3358,7 +3358,8 @@ TypedExpression FunctionEmitter::MaybeEmitCombinatorialValue(
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 @@ TypedExpression FunctionEmitter::MakeVectorShuffle(
}
}
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 @@ TypedExpression FunctionEmitter::MakeNumericConversion(
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 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) {
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 @@ bool FunctionEmitter::EmitImageQuery(const spvtools::opt::Instruction& inst) {
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 @@ bool FunctionEmitter::EmitImageQuery(const spvtools::opt::Instruction& inst) {
// 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 @@ TypedExpression FunctionEmitter::MakeOuterProduct(
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 @@ bool FunctionEmitter::MakeVectorInsertDynamic(
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 @@ bool FunctionEmitter::MakeCompositeInsert(
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>(

View File

@ -221,7 +221,7 @@ TEST_F(SpvParserTest_Composite_Construct, Struct) {
__struct_S
{
TypeConstructor[not set]{
__struct_S
__type_name_S
TypeConstructor[not set]{
__vec_2__f32
ScalarConstructor[not set]{50.000000}
@ -832,6 +832,23 @@ TEST_F(SpvParserTest_CompositeInsert, Struct) {
EXPECT_TRUE(fe.EmitBody()) << p->error();
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
@ -889,6 +906,30 @@ TEST_F(SpvParserTest_CompositeInsert, Struct_DifferOnlyInMemberName) {
EXPECT_TRUE(fe.EmitBody()) << p->error();
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
@ -915,6 +956,43 @@ VariableDeclStatement{
}
}
}
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{
@ -985,6 +1063,23 @@ TEST_F(SpvParserTest_CompositeInsert, Struct_Array_Matrix_Vector) {
EXPECT_TRUE(fe.EmitBody()) << p->error();
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

View File

@ -412,7 +412,7 @@ TEST_F(SpvParserTestMiscInstruction, OpUndef_InFunction_Struct) {
__struct_S
{
TypeConstructor[not set]{
__struct_S
__type_name_S
ScalarConstructor[not set]{false}
ScalarConstructor[not set]{0u}
ScalarConstructor[not set]{0}

View File

@ -470,7 +470,7 @@ TEST_F(SpvParserTest, EmitFunctionVariables_ArrayInitializer_Alias) {
__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 @@ TEST_F(SpvParserTest, EmitFunctionVariables_ArrayInitializer_Alias_Null) {
__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 @@ TEST_F(SpvParserTest, EmitFunctionVariables_StructInitializer) {
__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 @@ TEST_F(SpvParserTest, EmitFunctionVariables_StructInitializer_Null) {
__struct_S
{
TypeConstructor[not set]{
__struct_S
__type_name_S
ScalarConstructor[not set]{0u}
ScalarConstructor[not set]{0.000000}
TypeConstructor[not set]{

View File

@ -1442,13 +1442,9 @@ ast::Variable* ParserImpl::MakeVariable(uint32_t id,
}
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 @@ TypedExpression ParserImpl::MakeConstantExpression(uint32_t id) {
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 @@ ast::Expression* ParserImpl::MakeNullValue(typ::Type type) {
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 @@ ast::Expression* ParserImpl::MakeNullValue(typ::Type type) {
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;

View File

@ -1411,7 +1411,7 @@ TEST_F(SpvModuleScopeVarParserTest, StructInitializer) {
__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 @@ TEST_F(SpvModuleScopeVarParserTest, StructNullInitializer) {
__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 @@ TEST_F(SpvModuleScopeVarParserTest, StructUndefInitializer) {
__struct_S
{
TypeConstructor[not set]{
__struct_S
__type_name_S
ScalarConstructor[not set]{0u}
ScalarConstructor[not set]{0.000000}
TypeConstructor[not set]{